Fix accelerator in tests/test_train.py
#595
Conversation
Fix typo in test docstring.
|
After looking in the test again new questions came up. It has 'RunIf(min_gpus=1)' and has 'gpu' in the function name. For me this seems inconsistent. Do i have a wrong understanding about whats happening here? |
tests/test_train.py
Outdated
| @@ -37,7 +37,7 @@ def test_train_fast_dev_run_gpu(cfg_train: DictConfig) -> None: | |||
| @RunIf(min_gpus=1) | |||
| @pytest.mark.slow | |||
| def test_train_epoch_gpu_amp(cfg_train: DictConfig) -> None: | |||
| """Train 1 epoch on GPU with mixed-precision. | |||
| """Train 1 epoch on CPU with mixed-precision. | |||
There was a problem hiding this comment.
Considering @RunIf(min_gpus=1) and the name of the function being test_train_epoch_gpu_amp, I would assume that it would make more sense to instead change cfg_train.trainer.accelerator and set it to gpu?
There was a problem hiding this comment.
i would go with this reasoning as well.
I pushed a commit changing that.
What about the pytest.mark.slow decorator?
The test does not do a fast_dev_run like the gpu test before, but is still run on gpu an should be therfore somewhat fast.
Do you have any thoughts on that?
There was a problem hiding this comment.
My rule of thumb was to mark as slow everything that runs for full epoch or more. I guess this will be fast on gpu and mnist but might not if someone uses bigger dataset. Overall I don't think it's worth to think about this detail too much
* use gpu instead of cpu
* change docstring to represent usage of gpu
* remove pytest.slow decorator
ashleve
changed the title
tests/test_train.py docstring
ashleve
changed the title
tests/test_train.py docstringtests/test_train.py
Fix typo in test docstring.
What does this PR do?
The test reads like it should and is executed on CPU.
Now the test docstring is consistend.
Before submitting
pytestcommand?pre-commit run -acommand?Did you have fun?
Indeed :)