Conversation
added 3 commits
September 14, 2026 11:05
- 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
- 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
Collaborator
Author
sdtemple
commented
Sep 14, 2026
Comment on lines
+831
to
+847
| @pytest.mark.parametrize( | ||
| "reg_alpha, n_features, hidden_layer_sizes, n_classes", | ||
| [ | ||
| (1e-1, 40, (100,), 2), | ||
| (1e-1, 400, (100,), 2), | ||
| (2, 40, (100,), 2), | ||
| (2, 400, (100,), 2), | ||
| (None, 40, (100,), 2), | ||
| (None, 400, (100,), 2), | ||
| (1e-1, 40, (100, 100,), 3), | ||
| (1e-1, 400, (100, 100,), 3), | ||
| (2, 40, (100, 100,), 3), | ||
| (2, 400, (100, 100,), 3), | ||
| (None, 40, (100, 100,), 3), | ||
| (None, 400, (100, 100,), 3), | ||
| ] | ||
| ) |
Collaborator
Author
There was a problem hiding this comment.
This is copy, paste, revise from the related, and identically named, test function in test_regression.py
sdtemple
commented
Sep 14, 2026
| """ | ||
| # shape: (n_samples, n_features) | ||
| X, Y = validate_data(self, X, y) | ||
| Y = _to_numpy_cpu_y(Y) |
Collaborator
Author
There was a problem hiding this comment.
I have had success at making as few changes as possible in the GFDLClassifier directly (only this 1 line). Most of the magic should happen in GFDL.
sdtemple
commented
Sep 14, 2026
Comment on lines
+1336
to
+1337
| def _to_numpy_cpu_y(y): | ||
| """Convert label array y to a 1D NumPy array on CPU.""" |
Collaborator
Author
There was a problem hiding this comment.
This is the new private utility function. This is AI disclosured in the PR head comment.
Collaborator
Author
|
@eiviani-lanl I recommend looking and reviewing #129 before this one. |
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
1 participant
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(),.predict()and.predict_proba()inGFDLClassifier. Outside of.fit()inGFDLClassifierthe predict method are not directly touched but resolved through.fit()and.predict()in the inheritance (#129). There is a test function similar to the one intest_regression.pyfor accuracy acrosstorchversusnumpynamespaces.I run into a lot of problems when I make too many changes to
.fit()inGFDLClassifierdirectly, like in how #115 uses the DLPACK for converting between namespaces and devices and dtypes. This PR makes only 1 line change in that method, which calls to a private utility function. As noted in #129, a lot of issues with existing sklearn API conformance tests on edge cases crop up when making conversions in the.fit()function to use theOneHotEncoder.There are
ruff checkissues remaining, that do not concern the changes made in this PR.ruff check --fixand manual edits were made for appropriate linting in the PR changes here.Checklist:
[] Is there a more elegant solution than the private utility functions?
[] Is there an sklearn decorator we could use in the test suite?
[] Would some of the test functions already be captured by an sklearn decorator?
[] Benchmark speed against NumPy
[] Do the activation and weight generation functions need to be in xp? Would that affect speed?
[] Can we lower the atol in the test_torch_matches_numpy function?
This passes locally (mac os) and on the clusters (linux) for me (outside of the known issues with
pinvstability for test function on linux)AI disclosure: it wrote the private utility function, followed by human review and editing. Test function was written by human (copy, paste, revise from previous PR).