Internalize RoleMappedSchema and implications thereof #1902
Conversation
TomFinley
requested review from
Ivanidzo4ka,
yaeldekel,
artidoro,
sfilipi,
Zruty0 and
eerhardt
| @@ -324,7 +325,7 @@ private List<CoefficientStatistics> GetUnorderedCoefficientStatistics(LinearBina | |||
| /// <summary> | |||
| /// Gets the coefficient statistics as an object. | |||
| /// </summary> | |||
| public CoefficientStatistics[] GetCoefficientStatistics(LinearBinaryModelParameters parent, RoleMappedSchema schema, int paramCountCap) | |||
| internal CoefficientStatistics[] GetCoefficientStatistics(LinearBinaryModelParameters parent, RoleMappedSchema schema, int paramCountCap) | |||
There was a problem hiding this comment.
internal [](start = 8, length = 8)
can this stay public?
There is no other way to get the coefficient stats. the other public method gives stats on the bias, only.
Looking on wheather we can refactor to remove the dependancy on the RoleMappedSchema.
There was a problem hiding this comment.
we should have a workitem to have this method public, if this gets checked in as is, and address that in 0.9
In reply to: 242656194 [](ancestors = 242656194)
There was a problem hiding this comment.
We may need to build a public surface to it, but it can't be this specifically. We'll have to figure something else out. Perhaps since you understand the problem and requirements, it might be best for you to write the sample? Any public API dealing with RoleMappedSchema is right out though.
TomFinley
force-pushed
the
tfinley/Role
branch
from
c8fbc09 to
0559615
Compare
ghost
locked as resolved and limited conversation to collaborators
Internalization of everything related to role mapped schema, which includes schema-bindable mappers, much of the internal infrasturcture of evaluators, and so forth. As usual the commits are structured in such a way as only one "group" of code gets internalized at a time, with only the current last commit internalizing
RoleMappedSchemaitself.