Related to #92 but a different surface: that one is the TabFM model constructor, this is the scikit-learn estimator wrapper.
TabFMClassifier and TabFMRegressor subclass BaseEstimator, but do not define _parameter_constraints. Modern scikit-learn runs its parameter validation from that attribute, so without it none of the estimator's parameters is ever validated — not by tabfm, and not by sklearn either.
I tested every numeric parameter at 0 and -1 on b15593e4c1111ddb5f4f30dd2957df2edbaa04ca. All 26 combinations are accepted:
n_estimators 0, -1 ACCEPTED
outlier_threshold 0, -1 ACCEPTED
max_num_features 0, -1 ACCEPTED
max_num_rows 0, -1 ACCEPTED
softmax_temperature 0, -1 ACCEPTED
batch_size 0, -1 ACCEPTED
num_folds_for_cv 0, -1 ACCEPTED
n_feature_crosses 0, -1 ACCEPTED
n_svd_features 0, -1 ACCEPTED
total_svd_pool 0, -1 ACCEPTED
nnls_beta 0, -1 ACCEPTED
calibration_lambda 0, -1 ACCEPTED
min_rows_for_single_val_split 0, -1 ACCEPTED
Some of these have consequences further in. softmax_temperature is #93 — a negative value silently inverts the prediction. n_estimators reaches np.ones(n_est) / n_est (lines 2649, 2651, 3624, 3626), so zero gives a divide. num_folds_for_cv drives the cross-validation split. batch_size becomes a stride.
Adding _parameter_constraints would be the idiomatic fix and would cover all of them at once, including #93. It also brings the estimators into line with what check_estimator expects, which may be worth having anyway since the class already advertises the sklearn interface.
I have not tried to enumerate what each bad value does downstream — several need a fitted model — so I am reporting the missing validation rather than claiming a specific failure for each.
Disclosure: I used an AI assistant to help find this. I ran the matrix myself.
Related to #92 but a different surface: that one is the
TabFMmodel constructor, this is the scikit-learn estimator wrapper.TabFMClassifierandTabFMRegressorsubclassBaseEstimator, but do not define_parameter_constraints. Modern scikit-learn runs its parameter validation from that attribute, so without it none of the estimator's parameters is ever validated — not by tabfm, and not by sklearn either.I tested every numeric parameter at
0and-1onb15593e4c1111ddb5f4f30dd2957df2edbaa04ca. All 26 combinations are accepted:Some of these have consequences further in.
softmax_temperatureis #93 — a negative value silently inverts the prediction.n_estimatorsreachesnp.ones(n_est) / n_est(lines 2649, 2651, 3624, 3626), so zero gives a divide.num_folds_for_cvdrives the cross-validation split.batch_sizebecomes a stride.Adding
_parameter_constraintswould be the idiomatic fix and would cover all of them at once, including #93. It also brings the estimators into line with whatcheck_estimatorexpects, which may be worth having anyway since the class already advertises the sklearn interface.I have not tried to enumerate what each bad value does downstream — several need a fitted model — so I am reporting the missing validation rather than claiming a specific failure for each.
Disclosure: I used an AI assistant to help find this. I ran the matrix myself.