Repository navigation
Fix the SEM criterion and link fit - #11
Merged
Merged
Conversation
The five independent ifs are one if/elif chain, so exactly one branch sets the performance and the Gini branch computes it once, on the validation or training rows.
The log-likelihood is computed on the training rows, so the penalty log(n) k must use their number, not the size of the whole sample.
When every complete training row of a feature carries the same label, the logistic regression cannot be fitted and the chain crashed. Use a constant link with probability one for that label instead.
p * (2y - 1) - y + 1 is p(y=1) where y=1 and 1 - p elsewhere. Say so with np.where in one helper shared by the continuous and categorical branches.
With validation=True the aic and bic criteria logged a validation likelihood but computed it on the training rows, so the best iteration was picked on training data. Use the validation rows.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five fixes in the SEM fit (
glmdisc/_fitSEM.py), one commit each:ifblocks of_calculate_criterionare oneif/elifchain.validation=True, the aic/bic likelihood is now computed on the validation rows. It was logged as a validation likelihood but computed on the training rows, so the best iteration was picked on training data. This changes which iteration is kept for the defaultbic+ validation setting._SingleClassLink, aLogisticRegressionsubclass sobest_linkkeeps working indiscretize) is used instead.y_p,p * (2y - 1) - y + 1, is a named helper,_likelihood_of_labels, shared by the continuous and categorical branches.np.in1dwas alreadynp.isinon master (#10).Each fix has a test that fails on the previous code. Full suite: 98 passed.