Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

softmax for fp16 with fp32 accumulator #14098

Merged
merged 7 commits into from
Feb 21, 2019
Merged

Conversation

szha
Copy link
Member

@szha szha commented Feb 8, 2019

Description

softmax for fp16 with fp32 accumulator

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • softmax for fp16 with fp32 accumulator

@szha
Copy link
Member Author

szha commented Feb 8, 2019

@ptrendx @DickJC123 I'd appreciate your review on this. Thanks.

Copy link
Member

@ptrendx ptrendx left a comment

Choose a reason for hiding this comment

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

log_softmax should output its result in fp32 - it is very easy to overflow there.

src/operator/nn/softmax-inl.h Outdated Show resolved Hide resolved
src/operator/nn/softmax-inl.h Outdated Show resolved Hide resolved
@szha
Copy link
Member Author

szha commented Feb 8, 2019

I don't think we can change the return type of log_softmax in minor release.

@szha
Copy link
Member Author

szha commented Feb 8, 2019

I will add an optional dtype option for this.

@vandanavk
Copy link
Contributor

@mxnet-label-bot add [Operator, pr-awaiting-review]

@marcoabreu marcoabreu added Operator pr-awaiting-review PR is waiting for code review labels Feb 8, 2019
@szha szha removed the pr-awaiting-review PR is waiting for code review label Feb 8, 2019
Copy link
Contributor

@larroy larroy left a comment

Choose a reason for hiding this comment

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

nice change. Are we actually testing that we have more accuracy with an accumulator of higher precission?

@szha
Copy link
Member Author

szha commented Feb 12, 2019

@larroy thanks. Accuracy would be tied to a model and a dataset and is thus harder to reason. The more immediate effect is it's harder to overflow when accumulating e^x.

@szha
Copy link
Member Author

szha commented Feb 12, 2019

I intend to change the PR so that GradUseInOut only happens when dtype is specified and different from input type.

src/operator/nn/softmax-inl.h Show resolved Hide resolved
src/operator/nn/softmax.cc Outdated Show resolved Hide resolved
src/operator/nn/softmax.cc Outdated Show resolved Hide resolved
@eric-haibin-lin
Copy link
Member

eric-haibin-lin commented Feb 15, 2019

LGTM. @ptrendx any other concerns? Would AMP skip the cast if softmax accumulation for fp16 inputs is done in fp32?

@eric-haibin-lin eric-haibin-lin merged commit 862cbc6 into apache:master Feb 21, 2019
@szha szha deleted the softmax_fp16 branch February 21, 2019 04:56
jessr92 pushed a commit to jessr92/incubator-mxnet that referenced this pull request Feb 28, 2019
* softmax for fp16 with fp32 accumulator

* return AType in kernel

* add dtype

* kernel

* grad use in-out only when dtype override

* simplify infer type

* address comments
jessr92 pushed a commit to jessr92/incubator-mxnet that referenced this pull request Feb 28, 2019
* softmax for fp16 with fp32 accumulator

* return AType in kernel

* add dtype

* kernel

* grad use in-out only when dtype override

* simplify infer type

* address comments
jessr92 pushed a commit to jessr92/incubator-mxnet that referenced this pull request Feb 28, 2019
* softmax for fp16 with fp32 accumulator

* return AType in kernel

* add dtype

* kernel

* grad use in-out only when dtype override

* simplify infer type

* address comments
drivanov pushed a commit to drivanov/incubator-mxnet that referenced this pull request Mar 4, 2019
* softmax for fp16 with fp32 accumulator

* return AType in kernel

* add dtype

* kernel

* grad use in-out only when dtype override

* simplify infer type

* address comments
vdantu pushed a commit to vdantu/incubator-mxnet that referenced this pull request Mar 31, 2019
* softmax for fp16 with fp32 accumulator

* return AType in kernel

* add dtype

* kernel

* grad use in-out only when dtype override

* simplify infer type

* address comments
haohuanw pushed a commit to haohuanw/incubator-mxnet that referenced this pull request Jun 23, 2019
* softmax for fp16 with fp32 accumulator

* return AType in kernel

* add dtype

* kernel

* grad use in-out only when dtype override

* simplify infer type

* address comments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants