Repository navigation
Fix correctness bugs across the core modules - #7
Merged
Merged
Conversation
log_odd was an integer array, so each float coefficient added to it was silently truncated toward zero and the drawn labels followed the wrong probabilities. Accumulate in a float array instead.
Draw exactly one label per observation instead of relying on the shape of the probability vector to set the output size.
generate_data(plot=True) drew every pair of features on the same axes, so the scatterplots overlapped into an unreadable cloud. Open a figure per pair, label its axes and show it without blocking, as the plot method does.
The np.float alias was removed in NumPy 1.24, so every call to gini raised AttributeError. Use the builtin float, and add the first tests of the Gini index and its DeLong variance against scikit-learn's AUC.
"not level < 1 and level > 0" parses as "(level >= 1) and (level > 0)", so 0 and negative levels were accepted and produced NaN intervals. Test that the level lies strictly between 0 and 1 instead.
When a continuous feature had a quantization level made only of missing values, its lower bound was NaN, nanmin matched nothing and indexing the empty result raised IndexError. Ignore NaN bounds and only drop the smallest one when some remain.
The error raised when the number of categorical columns does not match the fitted model talked about predictors_cont, pointing users at the wrong argument. Fix the message for both the SEM and NN checks.
best_reglog is None before fit, which scikit-learn's check_is_fitted rejects with a TypeError, so predict and discretize on a new instance did not raise the documented NotFittedError. Test for None first.
m_start=2.5 passed the range check and crashed later in range(), and burn_in was never checked. Require integers, and a non-negative burn_in.
The GAM input was built from both predictors_cont and predictors_qual unconditionally, so a model fitted on continuous features only, or on categorical ones only, crashed on the missing block. Build the design from the blocks that exist.
plt.subplots returns a bare Axes rather than an array when there is a single subplot, so enumerate(axs) raised TypeError. Ask for a 2D array of axes.
The encoder of every categorical feature was fitted four times per feature to recompute its number of levels, in addition to the fit that is stored in affectations. Read the level count from the stored encoder.
For a categorical feature with fewer levels than m_start the initial labels were drawn from range(max_label), which left out the last level and made np.random.choice fail on an empty list for a one-level feature. Draw from the right number of levels with np.random.randint, which also replaces the list-based choice for the continuous features.
_initialize_neural_net fitted a fresh LabelEncoder per categorical feature just to read its largest label, although fit already stored one in affectations. Read it from there.
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.
First stream of correctness fixes across the core modules, as 15 small commits
that each keep the tests green and carry their own test.
generate_data: the log-odds were an integer array, so float coefficientswere truncated; labels are drawn with an explicit
size;plot=Trueopensone figure per feature pair instead of overplotting.
gini_utils:np.float(removed from NumPy) replaced byfloat, soginiworks again; the
levelcheck accepted 0 and negative values.best_formula: no moreIndexErrorwhen a level only holds missing values.discretize: the shape error for categorical columns namedpredictors_cont.Glmdisc.__init__: an unfitted SEM model raisesNotFittedErrorinstead ofTypeError;m_startandburn_inmust be integers.plot:logoddworks with only continuous or only categorical features andwith a single feature; last French comment translated.
fit: the initial labels are drawn withnp.random.randintfrom the rightnumber of levels (the last level was never used and a one-level feature
crashed); each
LabelEncoderis fitted once instead of four times perfeature, in
_init_discand in the NN initializer.Tests: 67 pass. The 24 tests in
tests/tests_NNfail exactly as they do onmaster (Keras 3 rejects
lr,backend.functionis gone); that is the subjectof a follow-up.
Not changed, noticed on the way:
_discretize_semlabels missing values withmax(new data) + 1, which depends on the scored batch; the NN initializerstill caps the number of groups at the number of levels minus one.