remove old features: amp, ddp, and rnn #1896
Conversation
crcrpar
commented
Apr 17, 2025
crcrpar
commented
- amp
- ddp
- RNN
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR removes legacy modules and features related to AMP, DDP, and RNN that are no longer maintained.
- Removes all AMP-related modules and utilities (including optimizer wrappers, state management, API functions, and documentation).
- Removes the deprecated RNN modules and associated components.
- Updates the top-level module exports and documentation to reflect these removals.
Reviewed Changes
Copilot reviewed 64 out of 64 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apex/amp/opt.py | Removed AMP optimizer wrapper implementation. |
| apex/amp/lists/torch_overrides.py | Removed AMP torch overrides. |
| apex/amp/lists/tensor_overrides.py | Removed AMP tensor overrides. |
| apex/amp/lists/functional_overrides.py | Removed AMP functional overrides. |
| apex/amp/handle.py | Removed AMP handle and scaling utilities. |
| apex/amp/compat.py | Removed AMP compatibility utilities. |
| apex/amp/amp.py | Removed core AMP API implementation. |
| apex/amp/_initialize.py | Removed AMP initialization logic. |
| apex/amp/_amp_state.py | Removed AMP state management. |
| apex/amp/version.py | Removed AMP version file. |
| apex/amp/init.py | Removed AMP package initializer. |
| apex/amp/README.md | Removed AMP documentation. |
| apex/init.py | Updated all to remove AMP references. |
| apex/RNN/models.py | Removed deprecated RNN models. |
| apex/RNN/cells.py | Removed deprecated RNN cells. |
| apex/RNN/init.py | Removed RNN package initializer. |
| apex/RNN/RNNBackend.py | Removed RNN backend implementation. |
| apex/RNN/README.md | Removed RNN documentation. |
| README.md | Updated top-level documentation to reflect removals. |
crcrpar
changed the title
Signed-off-by: Masaki Kozuki <[email protected]>
| return stackedRNN(inputRNN, num_layers, dropout = dropout) | ||
|
|
||
|
|
||
| def LSTM(input_size, hidden_size, num_layers, bias=True, batch_first=False, dropout=0, bidirectional=False, output_size = None): |
There was a problem hiding this comment.
I love that there's a deprecated_warning for all this code! Thanks for doing that a while ago, and double thanks for revisiting this now.
Rather than removing symbols outright, though, I'd recommend having a period where the implementations raise NotImplementedError. This gives time for the ecosystem to both update its code and, more importantly, update imports.
As a quick background: https://github.com/search?q=%22import+apex.amp%22+AND+path%3A%2F.py%24%2F&type=code shows 350 hits of projects import apex.amp, and of course people could also be from apex import amping. Most of that code will fail to even import after this change, even if they have conditionals that cause it to never actually use apex's amp at runtime anyway.
(I realize the search is for amp and i'm commenting on an LSTM symbol here; my comment is global, not specific to this symbol!)
There was a problem hiding this comment.
The idea does sound reasonable but as the message of deprecation warning says it'd be removed by the end of Feb. 2023. Thus I think it's high time we removed the features
There was a problem hiding this comment.
🤷 I'm not downvoting or upvoting, just recommending. You know the usage / space better than I do---I could be totally off on how quickly people update their code!
TestAmp failures observed on ROCm due to: deepspeedai@53e91a0#diff-e6635a81d2c2bf0938b5f83b1c4945e0f344e0106191a6960df8ee4aa64cc55fR2305 Since amp is deprecated and removed from apex (NVIDIA/apex#1896), and these tests are already skipped on NVIDIA, we will skip them on ROCm as well. Signed-off-by: Artem Kuzmitckii <[email protected]>
TestAmp failures observed on ROCm due to: 53e91a0#diff-e6635a81d2c2bf0938b5f83b1c4945e0f344e0106191a6960df8ee4aa64cc55fR2305, but since amp is deprecated and removed from apex (NVIDIA/apex#1896), and these tests are already skipped on NVIDIA, we will skip them on ROCm as well. Command for verification: `pytest -v tests/unit/runtime/half_precision/test_fp16.py -k 'TestAmp'` --------- Signed-off-by: Artem Kuzmitckii <[email protected]> Co-authored-by: Olatunji Ruwase <[email protected]>
2 participants
