Repository navigation
Conversation
- staging ground to have array api use - testing for torch cpu only right now - testing regressor only right now - test to match numpy and torch r2 score - test that predictions are in same namespace - many issues with sklearn conformance in classifier - more details to come about onehotencoder issues
- Two private utility functions for the following: - Check and/or convert all to X's dtype, namespace, & device - Ensure that X is of a float type - Test function to ensure predictions have same context as X - Test function that fitted attributes have same context as X - Test function that ints are handled gracefulyl
- Replace custom private utility functions with move_to - Is existing utility from sklearn - Achieves same functionality and more - array_api_compat no longer used in imports - Torch download removed from TOML - Previous tests pass pre PR (on local Windows) - Torch matches NumPy test passes - Other new tests related to namespace, dtype, device fail - Those will be replaced in next commit with sklearn equiv - Remaining ruff errors not related to this PR scope
- Replace np with xp in some activations - I think scipy uses array API standard - When SCIPY_ARRAY_API is set - Hence no change to scipy.special funcs - Responds to eiviani-lanl comment - No speed change from np.tanh to xp.tanh
- xp.maximum requires both to be Tensor - 0 as scalar only works for np - numpy not needed in activations.py - use sklearn way to get namespace
- Control the dtypes with `xp.astype` - `xp.astype` is used >200 times in sklearn - `move_to` is used over >200 times in sklearn - I.e., sklearn learn uses these for device, dtype - `conftest.py` sets global behavior for array API fixture - sklearn tag tells check estimator that array API is supported - All `test_regression.py` functions pass on local Mac OS - Custom tests are yet to be removed - Seeking alternative existing tests from sklearn to replace
* Added `check_array_api_input_and_values` function. This is a public utility that is not covered by `check(estimator)` and will verify if public attributes match to NumPy runs given flexible choice of array namespace. * I checked many combinations over `GFDLRegressor` inputs except for Torch and NumPy seem to handle backends of bounded weight schemes with slight numerical differences. * `check_array_api_input_and_values` does not have enough flexibility like `rtol` or `atol` to control slight backend differences * Modified `test_sklearn_api_conformance` to check both array API and non-array API version. Made a pytest.xfail for array API when _NotAnArray is put in. This is an sklearn specific object that is incompatible with the array API tests. * Two of my custom tests were already covered by `test_sklearn_api_conformance` with the `array_api_dispatch=True` flag * sklearn.linear_model favors `check_same_namespace` as function to ensure `.predict` input matches the `.fit` input * I liked the idea of converting to what the user puts into `.predict` but I will revert to the standard that sklearn has set * This removes `move_to` instances in `GFDL().predict` * For the time being, I commented out a custom test about integer arrays. sklearn does not address this case.
* Removed conftest.py file which globally set some array API settings * Reverted back to original check_estimator test * Made separate check_estimator test for array API Includes skips if array API not set up * Use pytest.importorskip for "torch" test * array_api_strict bawks at hidden_layer_sizes.min() * Removed array_api_compat dependency
* `check_array_api_input_and_values` uses `make_classification` which is unstable for some weight schemes between torch, np * Shrunk number of cases for that test which calls `check_array_api_input_and_values` * Added custom `make_regression`-based test that checks public attributes and predictions
* Applied to `hidden_layer_sizes`
* Implemented argument array_namespace instead of torch only * Used `clone()` in custom test to cut redundancy. This is what `sklearn` does in the related test `_check_array_api_core`. * Removed the 3x for loop of different random states. * The custom `xp_make_regression` test remains but ideally it could be removed if we could get an MR change through `sklearn` that allows for control of the simulating data.
* In `test_xp_make_regression` we need to use `with config_context()` otherwise `sklearn` utilities revert to NumPy. This was a bug. * No handling of dtype for the time being. All tests are xp.float64. * Needed to control `atol` and `rtol` * `move_to` to `assert_allclose` on same namespace (NumPy) * Small code change to use the array_api_compat vendor of the other namespaces (PyTorch)
* `check_array_api_input` only
* Values are evaluated with custom test that is
based on `check_array_api_core` but
gives control over simulating data,
`atol` and `rtol`
* Removed `pytest.importorskip("torch")`
* Instead, use the utility from sklearn
`yield_namespace_device_dtype_combinations()`
* `if` block to scope to torch, cpu, float64
* Don't force to max dtype precision
* All `WEIGHTS.keys()` and `ACTIVATIONS.keys()`
* AI disclosure: I consulted AI but wrote the small edits myself * ridge.coef_.T not strict compliant for 1-d arrays. Revised to not call .T if the array is 1-d * Revert from xp.asarray to np.asarray for hidden_layer_sizes. GaussianNB and others in sklearn have precedent for not all arrays being in xp. This just loops over layer sizes. In experimentation, I find that if this is xp.asarray it creates issues for weight sampling in cupy. No diff from main. * Shortened and revised some comments * Shrunk num of pytest marks by 2 * Added dtype_name to sklearn utility invocation
* AI disclosure: I did not use AI for this * Skip on `check_array_api` for classifier tests * We will remove those skips once we address the classifier in PR lanl#130
* SI disclosure: I did not use SI for this. This is my first set up of a CI, so hopefully it works smoothly. * Added new YML file with SCIPY_ARRAY_API=1 as the environment file * Only macos and ubuntu and python 3.14 * Only torch and array-api-strict installed
* Replace SciPy version of `log_sigmoid` and `sigmoid` with an `xp` version * `log_sigmoid` and `sigmoid` fail some array-api-strict regression tests. It is known from SciPy that `log_expit` is not yet supported. * Update `atol` and `rtol` values for `float32` tests. This is a pain identifying `atol` and `rtol` values suitable for 1e4 tests across platforms, backends, devices, and dtypes. The vast majority of tests pass with stricter than `atol=1e-0` and `rtol=1e-2` for `float32`. For tests that wouldn't pass a stricter threshold, I have verified that the coeffs are qualitatively the same (often a big coefficient 1e2 that may be off by a little over 1e-1). You can also inspect the singular values (manually) to see that they are also qualitatively the same.
* SI disclosure: I did not use SI * Implemented `xp` for base estimator's `partial_fit` method * Be wary of checking namespace without running `.fit()`, hence why you do not put this in `partial_fit` * First draft at adding `partial_fit` boolean to the custom test function * `xp.linalg.lstsq` does SVD as in `Ridge().fit()` but is not supported currently
* SI disclosure: I did not use SI * Replaced `scipy.special` versions of softmax functions with `xp` versions based on Emma's `sklearn` pull request * Updated `atol` and `rtol` bounds for tighter control in `xp_make_regression` test * Added quantitative comments about why we can't tighten further without failing the test (very few cases out of 720 tests over configurations) * We expect some classifier tests to fail, as the classifier will be addressed in lanl#130 which is a descendant of this PR. The vast majority that fail right now concern `partial_fit` in the `GFDLClassifier`
* Removed the `_NotAnArray` try/except block. The `sklearn` compliance function already handles which tests to run if SCIPY_ARRAY_API=1 * Removed the related test
* SI disclosure: I did not use SI, except I did use GPT 6.1 Sol when reviewing Emma's work * There were some Array API styles that Emma used in her sklearn PR that I preferred over mine. I also identified some redundant tests in my sklearn PR review. Here I am porting over those suggested changes. * eiviani-lanl/scikit-learn#2 * Use `xp.asarray` instead of `move_to` and `xp.astype` * Use `dtype=[xp.float64, xp.float32]` in `validate_data` * Only `xp.squeeze` when needed. Fixes a bug with classifier which will go upstream when I rebase lanl#130 * Removed redundant tests which were identified during review
* SI disclosure: I used SI to write a simple `partial_fit` test function * Replaced `numpy.testing.assert_allclose` with `xpx.testing.assert_close` for array API tests * Decreased number of Array API test cases
* SI disclosure: I did not use SI in this commit * Copy and pasted the `xp.asarray` set up for the `self.W_` and `self.b_` initialization to `partial_fit` * Simplified some lines to decrease line number
- private utility function for OneHotEncoder - one line call to it in .fit() - GFDLRegressor only has to invoked base GFDL - GFDLClassifier has that 1 line change in .fit() - Test function for numpy vs torch matching - Copy, paste, modify from test_regression.py - Other test functions in regressor PR would be redundant
* Order of `unique_labels` mattered for conformance * Test with larger dataset for `make_classification`. Tyler doesn't like this test but for the commit it confirms similar results for torch versus numpy for ALL weight schemes * Change .predict_proba and .predict in classifier to xp array versions * scipy.special.logsumexp is supported for array API * 6 tests fail with array-api-strict versus 0 for compat. Issue appears at `_to_numpy_cpu_y` * We want to remove that private utility eventually
* AI disclosure: I consulted AI but wrote the edits myself * Copy, paste, and modify the tests from `test_regression.py` in lanl#129 * Use `move_to` on the `self.classes_` * `check_same_namespace` called individually in each estimator's methods
* SI disclosure: I did not use SI for this * Main edit to to put `classes` in `predict` on the same namespace as `X` so that you can index on `xp.argmax` * Revise `xp_make_classification` test to allow for `partial_fit` results
* SI disclosure: I did not use SI * Consistent with the classifier `.fit`, we want `classes` to be on `np` and `cpu` for `OneHotEncoder` if this is the first pass when the encoder is fit.
* `xp.linalg.solve` documation for `partial_fit` in non-ensemble GFDL estimators * `softmax` from @activations.py achieves the same thing as in `partial_fit` custom written with `scipy.special.logsumexp` * These would eventually need to be addressed in an ensemble branch in the same way * Reverted to `rtol=1e-3` for `test_partial_fit` which had `pinv` instability in `EnsembleGFDLClassifier`. It looks like Tyler will `xfail` this case in lanl#138
* SI disclosure: I did not use SI, but the basis of the changes from review of Emma's PR did use GPT 6.1 Sol * Use `LabelBinarizer` instead of `OneHotEncoder`. The `OneHotEncoder` is designed for features, not labels. See the `sklearn` documentation. The `LabelBinarizer` has Array API support, which means we don't have to do device transfers all of the time. * Add `ValueError` if only 1 class is specified in a classification problem. Had to modify the `np.zeros` to `np.arange() % 2` in an invalid activation/weight test so that the `ValueError` doesn't trigger first. * `rtol` spec in a `np.assert_allclose` test
* SI disclosure: I did not use SI * Simplify/shorten some code lines in classifier fit test * Use `xpx.testing.assert_close` instead * Include a partial fit Array API test for classifier which only checks `predict` and `predict_proba`
* SI disclosure: I did not use SI * Include `n_classes` as a parameter for Array API classifier test
- private utility function for OneHotEncoder - one line call to it in .fit() - GFDLRegressor only has to invoked base GFDL - GFDLClassifier has that 1 line change in .fit() - Test function for numpy vs torch matching - Copy, paste, modify from test_regression.py - Other test functions in regressor PR would be redundant
* Order of `unique_labels` mattered for conformance * Test with larger dataset for `make_classification`. Tyler doesn't like this test but for the commit it confirms similar results for torch versus numpy for ALL weight schemes * Change .predict_proba and .predict in classifier to xp array versions * scipy.special.logsumexp is supported for array API * 6 tests fail with array-api-strict versus 0 for compat. Issue appears at `_to_numpy_cpu_y` * We want to remove that private utility eventually
* AI disclosure: I consulted AI but wrote the edits myself * Copy, paste, and modify the tests from `test_regression.py` in lanl#129 * Use `move_to` on the `self.classes_` * `check_same_namespace` called individually in each estimator's methods
* SI disclosure: I did not use SI for this * Main edit to to put `classes` in `predict` on the same namespace as `X` so that you can index on `xp.argmax` * Revise `xp_make_classification` test to allow for `partial_fit` results
* `xp.linalg.solve` documation for `partial_fit` in non-ensemble GFDL estimators * `softmax` from @activations.py achieves the same thing as in `partial_fit` custom written with `scipy.special.logsumexp` * These would eventually need to be addressed in an ensemble branch in the same way * Reverted to `rtol=1e-3` for `test_partial_fit` which had `pinv` instability in `EnsembleGFDLClassifier`. It looks like Tyler will `xfail` this case in lanl#138
* SI disclosure: I prompted the Claude Sonnet SI to make appropriate changes to `EnsembleGFDLClassifier` in alignment with the patterns of `GFDLClassifier` and `GFDLRegressor`. I then reviewed it. * I noticed that we could use `softmax` from @activations.py to replace a line in `predict_proba`. * Had to add a custom function to address the equivalent of `check_same_namespace` for `coeffs` list * Extended `xp_make_classification` test in @test_model.py
* SI disclosure: I used Claude Sonnet 5 to convert changes to `GFDLClassifier` that use `LabelBinarizer` instead of `OneHotEncoder` to the `EnsembleGFDLClassifier`. I reviewed that the changes (mostly copy and paste) and confirmed with `pytest` and `ruff` * This commit uses `LabelBinarizer` in `EnsembleGFDLClassifier` to avoid repeatedly making device transfers. `LabelBinarizer` is Array API compatible and designed for targets. `OneHotEncoder` is designed for features. * Removed `partial_fit` Array API test coverage
* SI disclosure: I did not use SI * Convert ensemble fit and partial fit to use `xp.asarray` * Add `EnsembleGFDLClassifier` to the partial fit Array API test function
Collaborator
|
Please add appropriate PR title prefixes, and probably WIP as well unless ready for final review. |
sdtemple
changed the title
sdtemple
changed the title
sdtemple
changed the title
GFDLEnsembleClassifier estimator
This branch has not been deployed
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.
This PR is scoped to address
.fit(),partial_fit(),predict()and.predict_proba()inEnsembleGFDLClassifier. Outside of.fit()andpartial_fit()inEnsembleGFDLClassifierthepredictandpredict_probamethods are not directly touched but resolved through.fit(),partial_fitand.predict()in the inheritance ofEnsembleGFDL.GFDLRegressor#129GFDLClassifier#130This edit is branched off from #130 (and has been rebased to it) and then adjusted to support Array API for
EnsembleGFDLClassifier. The new tests intest_model.pyhave been extended to cover the ensemble estimator. We have tested on NumPy, Array API Strict, PyTorch, and CuPy with CPU, MPS, and CUDA devices and float32 and float64 dtypes.To test, you would use an incantation like the following:
python -m pip install -e . python -m pip install array-api-strict array-api-compat torch cupy-cuda13x SCIPY_ARRAY_API=1 PYTORCH_ENABLE_MPS_FALLBACK=1 python -m pytestSI disclosure: I initially prompted Claude Sonnet 5.5 to make the same Array API changes to the ensemble estimator as
GFDLClassifier. Thereafter, I manually made changes after rebases on #130.