[BUGFIX] fix issue in elemwise_add

[KexinFeng]

Description

Thie PR is created to fix the issue #20293, where operator elemwise_add has bug when CachedOp uses static_alloc.

Checklist

Essentials

• PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
• Changes are complete (i.e. I finished coding on this PR)
• All changes have test coverage
• Code is well-documented

Changes

• Feature1, tests, (and when applicable, API doc)
• Feature2, tests, (and when applicable, API doc)

Comments

• If this change is a backward incompatible change, why must this change be made.
• Interesting edge cases to note here

[mxnet-bot]

Hey @KexinFeng , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:

• To trigger all jobs: @mxnet-bot run ci [all]
• To trigger specific jobs: @mxnet-bot run ci [job1, job2]

---

CI supported jobs: [unix-gpu, edge, clang, centos-gpu, sanity, centos-cpu, windows-cpu, website, windows-gpu, miscellaneous, unix-cpu]

---

Note:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.

[matteosal]

I have verified that this change fixes the issue

[ptrendx]

This is unfortunately not a valid fix for the issue (and the fact that without static allocation everything works fine shows that) - the bug seems to lie rather inside the CachedOp and its handling of the static memory allocation. The CloneGradient method in the backward of elemwise_add is just a simple passing the input gradient data in both directions (vs copying it as the proposed ElemwiseGradUseNone does), which is the right thing to do.

[matteosal]

Any news on this?

This is unfortunately not a valid fix for the issue (and the fact that without static allocation everything works fine shows that) - the bug seems to lie rather inside the CachedOp and its handling of the static memory allocation. The CloneGradient method in the backward of elemwise_add is just a simple passing the input gradient data in both directions (vs copying it as the proposed ElemwiseGradUseNone does), which is the right thing to do.

So this is an inefficient workaround because it wastes some time copying the data? Can there be other shortcomings besides performance by using this fix? We really need this to work and we don't mind paying just a small performance drop while waiting for a proper fix.

[KexinFeng]

Any news on this?

This is unfortunately not a valid fix for the issue (and the fact that without static allocation everything works fine shows that) - the bug seems to lie rather inside the CachedOp and its handling of the static memory allocation. The CloneGradient method in the backward of elemwise_add is just a simple passing the input gradient data in both directions (vs copying it as the proposed ElemwiseGradUseNone does), which is the right thing to do.

So this is an inefficient workaround because it wastes some time copying the data? Can there be other shortcomings besides performance by using this fix? We really need this to work and we don't mind paying just a small performance drop while waiting for a proper fix.

Hi, currently this fix can deal with your problem but fails other tests during the pull request. So it is still under investigation. It doesn't seem to me to cause inefficiency; it's just not valid yet. But maybe it can be a workaround