aimclub / aimclub/FEDOT

[Bug]: incomplete `xgbreg` -> `xgboostreg` rename from #1209 leaves five stale references

オープン
#1,452 コメント 3 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Python
スター
712
フォーク
96
平均マージ
3日 1時間
マージ済み PR(30日)
10

説明

## Summary

The `xgbreg` → `xgboostreg` rename introduced in #1209 (`80eba8ef`, 2024-07-26)
was applied to `model_repository.json` but not to five other places that still
refer to the old name. `xgbreg` is not defined in any operation repository, so
each of those references is unreachable.

One of them is not cosmetic: `xgboostreg` is the only boosting operation with no
entry in `HyperparametersPreprocessor.all_preprocessing_rules`.

## Scope

- [ ] `fedot/core/operations/hyperparameters_preprocessing.py:40` — rules
stranded under the old key; `xgboostreg` receives none
- [ ] `fedot/core/operations/evaluation/evaluation_interfaces.py:174–196` —
five unreachable strategy entries, three imports kept alive only by them,
and the docstring that mirrors them
- [ ] `fedot/api/builder.py:308,360` — public API docstring lists an operation
that cannot be resolved
- [ ] `fedot/structural_analysis/operations_hp_sensitivity/params_bounds.json:144`
— parameter bounds keyed on the old name
- [ ] `test/data/model_repository.json:190` — fixture still declares `xgbreg`,
and `xgboost` under `sklearn_class`

## Plan

Unless you would rather handle it differently, I would prepare this as two pull
requests:

1. `chore:` — items 2, 3 and 5. Statically provable, no behaviour change.
2. `fix:` — items 1 and 4, restoring the rules and bounds under `xgboostreg`,
with a unit test covering the coercion.

Splitting them keeps the reviewable part independent of the one that changes
behaviour. Happy to merge them into one PR, drop any item, or leave the whole
thing to the team if it is already on someone's plate.

---

## Details

### The rename

```
$ grep -l '"xgbreg"' fedot/core/repository/data/*.json
(no matches)
$ grep -n '"xgboostreg"' fedot/core/repository/data/model_repository.json
537: "xgboostreg": {

$ git show 80eba8ef -- fedot/core/repository/data/model_repository.json | grep -E '^[+-].*xgb'
- "xgbreg": {
+ "xgboostreg": {
```

The commit message of #1209 states the intent: *Xgboost migrated from
SkLearnEvaluationStrategy to separate BoostingStrategy.* The commit touches
neither `evaluation_interfaces.py` nor `hyperparameters_preprocessing.py` nor
`params_bounds.json`.

### 1. Hyperparameter preprocessing rules

`HyperparametersPreprocessor` runs on every operation fit (`operation.py:45`)
and selects rules by exact operation name:

```python
return HyperparametersPreprocessor.all_preprocessing_rules.get(operation_type, {})
```

Checking its keys against the operations declared across the four repository
files:

```python
import json, re
ops = set()
for f in ('model', 'data_operation', 'automl', 'gpu_models'):
ops |= set(json.load(open(f'fedot/core/repository/data/{f}_repository.json'))['operations'])
src = open('fedot/core/operations/hyperparameters_preprocessing.py').read()
keys = set(re.findall(r"^ '([a-z_0-9]+)': \{", src, re.M))
print('keys with no matching operation:', sorted(keys - ops))
```

```
keys with no matching operation: ['xgbreg']
```

One stale key out of nineteen. Every other boosting operation is present —
`xgboost`, `lgbm`, `lgbmreg`, `catboost`, `catboostreg` — which is what suggests
an oversight rather than a deliberate removal. The stranded rules coerce
`nthread`, `n_estimators`, `max_depth`, `max_leaves` and `max_bin` to integers.

`FedotXGBoostImplementation.check_and_update_params` does validate parameters,
but only `early_stopping_rounds`, `use_eval_set`, `booster` and
`enable_categorical`; it does not perform that coercion.

I have not measured the practical impact — `search_space.py:137` may already
supply integer values for `xgboostreg` during tuning, in which case the effect
is latent rather than active. Either way the asymmetry with the other five
boosting operations looks unintended.

This file was revisited in #1343 (2025-03-28, *preprocessing hotfixes*) and the
stale key was not noticed then.

### 2. Unreachable entries in `SkLearnEvaluationStrategy`

`_operations_by_types` holds 21 keys. Resolution goes operation → `meta` →
`strategies` in the repository metadata, and only 16 are reachable that way, via
`sklearn_class` and `sklearn_regr`. The remaining five are not:

| Key | Why unreachable |
|---|---|
| `xgbreg` | no such operation exists |
| `xgboost`, `lgbm`, `lgbmreg` | `meta` is `boosting_class` / `boosting_regr`, handled by `BoostingStrategy` since #1209 |
| `kmeans` | `meta` is `sklearn_clust` → `SkLearnClusteringStrategy`, which overrides the dict with its own `kmeans` entry |

`SkLearnClassificationStrategy` and `SkLearnRegressionStrategy` do not override
the dict and share this one. `SkLearnClusteringStrategy` and
`CuMLEvaluationStrategy` override it in full, so neither inherits the dead keys.

Those five entries are the only reason the module imports XGBoost and LightGBM
at import time:

```python
from lightgbm.sklearn import LGBMClassifier, LGBMRegressor # line 7
from sklearn.cluster import KMeans as SklearnKmeans # line 8
from xgboost import XGBClassifier, XGBRegressor # line 28
```

None of those names appears anywhere else in the file. Both packages stay
required by `boostings_implementations.py`, so no dependency changes — this only
removes an eager import from a module most of the operation layer pulls in. The
class docstring at lines 148–168 mirrors the same five mappings.

### 3. Public API docstring

`api/builder.py:308` and `:360` both list ``xgbreg`` among available operations.
A user copying that name into `available_operations` gets an operation no
repository can resolve.

### 4. Sensitivity analysis bounds

`params_bounds.json:144` is keyed on `xgbreg`. Last touched in #1114, before the
rename.

### 5. Test fixture

`test/data/model_repository.json:190` declares `xgbreg` under `sklearn_regr` and
`xgboost` under `sklearn_class`, both outdated. The fixture is read only by
`_model_metadata_example` in `test/integration/models/test_repository.py`, which
inspects the metadata of the first key, so nothing currently depends on these
two entries.

---

## Noticed nearby, not part of this

`BoostingStrategy` declares `__operations_by_types` with name mangling while the
base class reads `_operations_by_types`. It works, since `BoostingStrategy` also
overrides `_convert_to_operation`, but `EvaluationStrategy._find_operation_by_impl`
would raise `AttributeError` on it. Can open separately if of interest.

## Environment

FEDOT at `26f3af03`, Python 3.10.21, Ubuntu 24.04 (WSL2).
`python -m pytest test/unit -q` → 647 passed.

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。