AutoYAHP Part 3: Refactor the models, datasets, and trainer_hparams #1072
Conversation
This file contains 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
- Treat all python warnings as errors in tests. Existing tests that were throwing warnings were either fixed (yay!) or modified with `@pytest.mark.filterwarnings` - In BlurPool, throwing an error if both `replace_maxpools` and `replace_convs` are False, as that results in the algorithm being a no-op. - Made the default optimizer warning in the trainer less verbose. - Converted the bert yaml to have duration specified in terms of samples, to fix an issue where the warmup period, combine with max duration, was a noop. - Moved `TestMetricSetter` to `tests/common` and renamed as `MetricSetterCallback`. Tests should not be importing from other test files (tests/common is OK) - Removed TestWandBLogger, since the trainer tests do the same thing
Fix progressive resizing
This PR refactors the algorithms and tests as will be required by AutoYAHP. It does not depend on AutoYAHP itself (a future PR will remove the underlying hparam classes). - Refactored algorithm tests to not depend on hparams - Reformatted the factorize and selective backprop docstrings so they would be correctly parsed by auto-yahp - Refactor `algorithm_settings.py` to not depend on hparams and to return a list of `pytest.param` objects for a `pytest.mark.parametrize`. This change makes it more re-usable since it now includes information about markers required for each algorithm. - Moved the `TestTrainerAlgorithms` into `tests/algorithms/test_algorithms_train.py`, since it tests the individual algorithms, not the trainer, and thus should live in `tests/algorithms`. - Add helper methods for scanning a module to discover subclass implementations, check that the registry contains an entry, and test that a class is constructable from yaml
Silencing deepspeed deprecation warnings
Bump pyright
A-Jacobson
reviewed
May 25, 2022
A-Jacobson
reviewed
May 25, 2022
Definitely not Yes, amp is the best performance but it could cause some training runs to overflow. If I'm training a model and getting |
A-Jacobson
approved these changes
May 25, 2022
eracah
approved these changes
May 25, 2022
Discussed offline; decided to go with option 4 (leave as is), as option 3 would require the yamls to specify |
…taset_trainer_for_autoyahp
…taset_trainer_for_autoyahp
…aicml/ravi-composer into refactor_callbacks_for_autoyahp
…taset_trainer_for_autoyahp
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 cleans up the model hparams, dataset hparams, and trainer hparams for AutoYAHP. It does not depend on AutoYAHP itself (a future PR will remove the underlying hparam classes).
DeviceGPU()/DeviceCPU()instances directly, andDeviceGPU()throws an error if cuda is not available (before, yahp would parse intoDeviceGPUHparams(), which could then be replaced withDeviceCPUHparams()). (While it would have also worked to patch the yaml directly, I thought it'd be generally better to make the yamls more portable).