Skip to content

Deprecate unused APIs#321

Merged
ksivaman merged 3 commits into
NVIDIA:mainfrom
ksivaman:deprecate_api
Jul 14, 2023
Merged

Deprecate unused APIs#321
ksivaman merged 3 commits into
NVIDIA:mainfrom
ksivaman:deprecate_api

Conversation

@ksivaman
Copy link
Copy Markdown
Member

No description provided.

Signed-off-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
@ksivaman ksivaman requested review from ptrendx and timmoon10 July 14, 2023 15:06
@ksivaman
Copy link
Copy Markdown
Member Author

/te-ci

Copy link
Copy Markdown
Collaborator

@timmoon10 timmoon10 left a comment

Choose a reason for hiding this comment

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

We should only display the warning in the case the user tries to use these APIs. Also, I think exceptions are more appropriate than warnings since we are not going to be following the behavior expected by users.

Comment thread transformer_engine/pytorch/module/linear.py Outdated
Comment thread transformer_engine/pytorch/module/linear.py Outdated
Comment thread transformer_engine/pytorch/module/layernorm_linear.py Outdated
Comment thread transformer_engine/pytorch/module/layernorm_linear.py Outdated
Signed-off-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
@ksivaman
Copy link
Copy Markdown
Member Author

@timmoon10 I agree that the exception makes more sense for when a weight and bias are passed in to forward, but I've left the skip_weight_param_allocation passed in as just a warning, which I think is better.

Copy link
Copy Markdown
Collaborator

@timmoon10 timmoon10 left a comment

Choose a reason for hiding this comment

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

LGTM. My last nitpick is that we could mention in the warning message that skip_weight_param_allocation is not doing anything.

Signed-off-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
@ksivaman
Copy link
Copy Markdown
Member Author

Pipeline 8976176

@ksivaman ksivaman merged commit 58d2eba into NVIDIA:main Jul 14, 2023
@ksivaman ksivaman deleted the deprecate_api branch July 19, 2023 01:38
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.

2 participants