-
Notifications
You must be signed in to change notification settings - Fork 235
[MNT] Add missing load_model test for deep clusterers (Fixes #3080) #3111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MNT] Add missing load_model test for deep clusterers (Fixes #3080) #3111
Conversation
Thank you for contributing to
|
hadifawaz1999
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking care of this issue. Two comments for now:
- The test should be for all models under clustering/deep_learning
- The test should use tempfile (see this for example)
hadifawaz1999
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to call predict because when "fit" is called on a deep clusterer, it creates an instance self._estimator for the clustering estimator that you can fetch after calling fit and give it to the load_model
I've updated the test as suggested:
All tests pass locally with TensorFlow enabled. |
|
LGTM ! thanks |
[MNT] Add missing load_model test for deep clusterers
Reference Issues/PRs
What does this implement/fix? Explain your changes.
This PR adds a missing unit test for the deep clustering autoencoder models (
AEDRNNClustererandAEDCNNClusterer) to ensure that theload_modelfunctionality works as expected.Issue #3080 identified that while deep clusterers support loading a pre-trained Keras model, this behavior was not covered by any tests. This lack of test coverage allowed a bug fixed in PR #3074 to remain undetected.
This PR introduces a new test file,
test_deep_clusterer_io.py, which:.kerascheckpoint is saved viasave_best_model=True.load_modelto load the saved model, including custom layers(
_TensorDilationfor AEDRNN and_WeightNormalizationfor AEDCNN).model_.The test does not call
predict()on the loaded estimator, because estimator stateis intentionally not restored by
load_modeland would correctly raiseNotFittedError.This matches the design of the deep clusterer classes.
This adds the required IO coverage to ensure regressions do not reoccur.
Does your contribution introduce a new dependency? If yes, which one?
No new dependencies are introduced.
The test is guarded by a TensorFlow soft dependency check.
It will be skipped if TensorFlow is not installed, consistent with other deep learning tests.
Any other comments?
test_deep_clusterer_io.py) to keep IO-specific behavior separate from existing feature and base tests.PR checklist
For all contributions
[MNT], matching aeon’s conventions.For new estimators and functions
For developers with write access