Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #1751 #1779

Merged
merged 2 commits into from
Apr 7, 2020
Merged

fix #1751 #1779

merged 2 commits into from
Apr 7, 2020

Conversation

qmpzzpmq
Copy link
Contributor

@qmpzzpmq qmpzzpmq commented Apr 7, 2020

register ctc loss_fn as float function
@ShigekiKarita

register ctc loss_fn as float function
@ShigekiKarita ShigekiKarita self-requested a review April 7, 2020 04:01
@ShigekiKarita
Copy link
Member

I have already fix this issue in #1777 for float16 training (not only for apex.amp). However, notifying apex.amp for ignoring CTC also looks good to me.

@ShigekiKarita
Copy link
Member

I found that apex.amp already register torch.ctc_loss as FP32_FUNCS. Can you check your local apex already done this?
https://github.com/NVIDIA/apex/blob/ca00adace51ae4d82d2ff5cfc1ef1c9174ff6aaa/apex/amp/lists/functional_overrides.py#L67

@qmpzzpmq
Copy link
Contributor Author

qmpzzpmq commented Apr 7, 2020

Yes, but I found only register the torch.ctc is not enough, there still some error, You can check it by your self.

@ShigekiKarita
Copy link
Member

OK. Please ignore circle CI errors. I think they are not related to this PR.

@ShigekiKarita ShigekiKarita added this to the v.0.6.3 milestone Apr 7, 2020
@ShigekiKarita ShigekiKarita linked an issue Apr 7, 2020 that may be closed by this pull request
Co-Authored-By: Shigeki Karita <shigekikarita@gmail.com>
@ShigekiKarita ShigekiKarita merged commit 454f750 into espnet:master Apr 7, 2020
@ShigekiKarita
Copy link
Member

Thanks!

@kamo-naoyuki
Copy link
Collaborator

amp.init() is old api.
https://nvidia.github.io/apex/amp.html#for-users-of-the-old-amp-api

and register_float_function should be done before initialize, but is amp.register_float_function really necessary? It seems meaningless if casting.

@qmpzzpmq qmpzzpmq deleted the fix1751 branch April 9, 2020 00:57
@kamo-naoyuki
Copy link
Collaborator

@qmpzzpmq Please fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

built in ctc conflict with apex
3 participants