Update sklearn models for feature probabilities - #365
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| def __init__(self, max_time: float | int = 1, seed: int = 0, weight_features_by_correlation: bool = False): | ||
| self.max_time = max_time | ||
| self.seed = 0 | ||
| self.weight_features_by_correlation = weight_features_by_correlation |
There was a problem hiding this comment.
Bug: Seed Parameter Not Applied: Always Zeroed Seed
The seed parameter is hardcoded to 0 instead of using the seed parameter passed to init. Line 86 should be self.seed = seed instead of self.seed = 0. This bug prevents users from setting a custom random seed, causing all instances to use seed=0 regardless of what value is passed to the constructor.
716e930 to
2ac3c6b
Compare
| if total <= 0: | ||
| return random.choice(self.options) | ||
| normalized = [w / total if w >= 0 else 0.0 for w in self.weights] | ||
| return random.choice_weighted(self.options, normalized) |
There was a problem hiding this comment.
Bug: Incorrect normalization with negative weights
Incorrect normalization when weights contain both negative and non-negative values. The code computes total = sum(self.weights) including negative weights, then normalizes by replacing negative weights with 0.0 and dividing non-negative weights by total. This results in normalized weights that don't sum to 1.0. For example, with weights [2.0, -1.0, 2.0], total=3.0, but normalized=[0.667, 0.0, 0.667] sums to 1.334. The correct approach is to filter negative weights first, compute the sum of remaining positive weights, then normalize.
| def wrapper(v:float) -> float: | ||
| return 1 - abs(float(np.average(v))) + 0.00001 | ||
|
|
||
| return [wrapper(np.corrcoef(data[:, i], target)) for i in range(len(feature_names))] |
There was a problem hiding this comment.
Bug: Incorrect correlation weight computation using full matrix
The correlation_weights method incorrectly uses np.corrcoef(data[:, i], target) which returns a 2x2 correlation matrix, not a scalar value. The wrapper function then calls np.average() on this matrix, computing the average of all 4 elements (which includes two 1.0 values on the diagonal and two correlation coefficients), resulting in an incorrect weight calculation. The correct approach would be to extract the correlation coefficient using indexing: np.corrcoef(data[:, i], target)[0, 1].
| Var.feature_names = feature_names # type:ignore | ||
| index_of = {n: i for i, n in enumerate(feature_names)} | ||
| Var.to_numpy = lambda s: f"dataset[:,{index_of[s.name]}]" # type:ignore | ||
| Var = weight(10)(Var) |
There was a problem hiding this comment.
Bug: Conditional weighting bypasses flag in get_grammar
The get_grammar method unconditionally calls self.correlation_weights(feature_names, data, target) and applies the weights, ignoring the weight_features_by_correlation parameter. The correlation-based weighting should only be applied when self.weight_features_by_correlation is True, otherwise uniform weights should be used.
| index_of = {n: i for i, n in enumerate(feature_names)} | ||
| Var.to_numpy = lambda s: f"dataset[:,{index_of[s.name]}]" | ||
| weights = self.correlation_weights(feature_names, data, target) | ||
| Var = make_var(feature_names, weights=weights, relative_weight=10) |
There was a problem hiding this comment.
Bug: Correlation weighting ignored when disabled.
The get_grammar method unconditionally calls self.correlation_weights(feature_names, data, target) and applies the weights, ignoring the weight_features_by_correlation parameter. The correlation-based weighting should only be applied when self.weight_features_by_correlation is True, otherwise uniform weights should be used.
1bbdf8e to
b3e5e92
Compare
|
|
||
| def wrapper(corr_value: float) -> float: | ||
| # Higher absolute correlation -> smaller weight (bias search), add epsilon | ||
| return 1 - abs(corr_value) + 0.00001 |
There was a problem hiding this comment.
Bug: Inverted Correlation Weighting Misaligns Sampling Likelihood
The correlation weighting logic is inverted. The documentation states features should be "sampled with probabilities proportional to their absolute Pearson correlation", but the implementation returns 1 - abs(corr_value), which gives LOWER weights to features with HIGHER correlation. For a feature with correlation 0.9, the weight becomes 0.1, while a feature with correlation 0.1 gets weight 0.9. This is inversely proportional to correlation, contradicting the intended behavior. The formula should be abs(corr_value) + 0.00001 instead.
1 participant
Note
Adds optional correlation-weighted feature sampling across sklearn estimators, improves robustness of weighted choice, updates docs, and enhances the examples runner with uv and optional Codon.
weight_features_by_correlationoption in baseGeneticEngineEstimator; compute feature probabilities viacorrelation_weights.make_var(..., weights=...)to bias terminal sampling; keep relative weight10.VarRangeWithProbabilitieswith computed weights andweight(10); updateHillClimbingClassifierinit and_parameter_constraints.make_varnow supports optional probability weights viaVarRangeWithProbabilities, retains backward compatibility with numericrelative_weight.choice_weightedsanitizes non-finite/non-positive weights and falls back to uniform when needed.f1_score(..., average="weighted").run_examples.sh: run withuvby default; optional Codon execution with heuristic fallback.Written by Cursor Bugbot for commit 82473f0. This will update automatically on new commits. Configure here.