Skip to content

Conversation

ravi-mosaicml
Copy link
Contributor

@ravi-mosaicml ravi-mosaicml commented May 20, 2022

Similar to #1056, this PR cleans up the callbacks and loggers for AutoYAHP. It does not depend on AutoYAHP itself (a future PR will remove the underlying hparam classes).

  • Refactored callback and logger tests to not depend on hparams
  • Reformatted the docstrings so they would be correctly parsed by auto-yahp
  • Added callback_settings.py, similar to algorithm_settings.py, to return a list of pytest param objects for parameterization across callback tests. These param objects include appropriate markers (e.g. conditional skipping for wandb and mlperf; requiring that the memory monitor runs on GPU, ...)
  • Moved the TestTrainerAssets into tests/callbacks/test_callbacks.py, since it tests the individual callbacks and loggers, not the trainer, and thus should live in tests/callbacks.
  • Cleaned up the MemoryMonitor warnings when cuda is not available. Now, it warns when the model is not on cuda, to catch the edge case where one does CPU training when GPUs are available.

- 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
Copy link
Contributor

@eracah eracah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good! Just had a few questions I wanted answered.
Thanks!

@ravi-mosaicml ravi-mosaicml requested a review from eracah May 25, 2022 22:21
@eracah
Copy link
Contributor

eracah commented May 25, 2022

Ok. LGTM

Copy link
Contributor

@eracah eracah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ravi-mosaicml ravi-mosaicml enabled auto-merge (squash) May 25, 2022 23:01
@ravi-mosaicml ravi-mosaicml merged commit 04b4a83 into mosaicml:dev May 26, 2022
@ravi-mosaicml ravi-mosaicml deleted the refactor_callbacks_for_autoyahp branch May 31, 2022 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants