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

Float64 fallback for mkldnn subgraph and rnn op #15853

Merged
merged 13 commits into from
Sep 19, 2019
Merged

Float64 fallback for mkldnn subgraph and rnn op #15853

merged 13 commits into from
Sep 19, 2019

Conversation

ZhennanQin
Copy link
Contributor

Description

This can resolve #15767.

@PatricZhao @TaoLv @ciyongch @reminisce

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • 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)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

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

@pengzhao-intel
Copy link
Contributor

@ZhennanQin could you try CI again?

@pengzhao-intel
Copy link
Contributor

@ZhennanQin @xinyu-intel please retrigger the CI

@ZhennanQin
Fix mkldnn subgraph with float64
88f59cd
@ZhennanQin
Fix ci
bc4d3b5 
Change-Id: I0bb4e8d7a0a534aa661601887cc633cb9c4fcadf
@ZhennanQin
Fix test
5ea26ed 
Change-Id: I96c529abe7adb6def90a22f03b3432263ef12fda
@ZhennanQin ZhennanQin mentioned this pull request Aug 27, 2019
Merged
@ZhennanQin
Update dmlc-core
6e912e6 
Change-Id: I472fb7bbffc16ed8c36494ab49838b08c59b2f12
@ZhennanQin ZhennanQin requested a review from szha as a code owner August 29, 2019 02:02
@ZhennanQin
Merge remote-tracking branch 'offical/master' into float64_fallback
001fd5e 
Change-Id: Iecfef9e30e8fecd3bcb97b336ece0116a204c6f1
@pengzhao-intel
Copy link
Contributor

@ZhennanQin could you resolve the conflict and I will merge this one?

pengzhao-intel
pengzhao-intel approved these changes Sep 5, 2019
View reviewed changes
Copy link
Contributor

@pengzhao-intel pengzhao-intel left a comment

Choose a reason for hiding this comment

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

Please change the dmlc-core to the official repo.

@ZhennanQin
Merge remote-tracking branch 'offical/master' into float64_fallback
3195ba2 
Change-Id: Ia39ea4e373a78a4dbf50861fe62dcd2bdcc2bea1
@ZhennanQin
pin to offical dmlc
48a6ef3 
Change-Id: I5a27dc83b892bf8fcb34bb089449d1d3b6e9beed
@ZhennanQin
Merge remote-tracking branch 'refs/remotes/offical/master' into float…
95f5078 
…64_fallback

Change-Id: Ib5afeb70cdf9cf20a0551a1a38fad93d70a486bb
@ZhennanQin
Merge remote-tracking branch 'offical/master' into float64_fallback
2c7e8f3 
Change-Id: I36cbffe0dcd391ad379abbe5641b2234fd9a0946
@ZhennanQin
Fix GPU CI
fc97b9d 
Change-Id: I285947e01bdb0651c2c7830ed4eb76931a09b754
@pengzhao-intel
Copy link
Contributor

@ZhennanQin Try CI again?

@ZhennanQin
Merge remote-tracking branch 'offical/master' into float64_fallback
fed2632 
Change-Id: Iced43c2e411ac663540ebf5c7c355b6e06a29775
@ZhennanQin
Fix GPU CI
6d3a6b3 
Change-Id: I6f23b51d6bda44f6ae18766ebe390118740bb9c7
@ZhennanQin
Merge remote-tracking branch 'offical/master' into float64_fallback
98e643e 
Change-Id: Ica49bc6e5eea4711ce36ee69215e9aabb93e84cc
@ZhennanQin
Copy link
Contributor Author

@pengzhao-intel Finally, finally, this got CI passed.

@pengzhao-intel
Copy link
Contributor

Thanks a lot, really difficult :(

Merging now.

@pengzhao-intel pengzhao-intel merged commit a37a76c into apache:master Sep 19, 2019
@ZhennanQin ZhennanQin deleted the float64_fallback branch September 20, 2019 00:36
larroy pushed a commit to larroy/mxnet that referenced this pull request Sep 28, 2019
@ZhennanQin @larroy
Float64 fallback for mkldnn subgraph and rnn op (apache#15853)
c034773 
* Fix mkldnn subgraph with float64

* Fix ci

Change-Id: I0bb4e8d7a0a534aa661601887cc633cb9c4fcadf

* Fix test

Change-Id: I96c529abe7adb6def90a22f03b3432263ef12fda

* Update dmlc-core

Change-Id: I472fb7bbffc16ed8c36494ab49838b08c59b2f12

* pin to offical dmlc

Change-Id: I5a27dc83b892bf8fcb34bb089449d1d3b6e9beed

* Fix GPU CI

Change-Id: I285947e01bdb0651c2c7830ed4eb76931a09b754

* Fix GPU CI

Change-Id: I6f23b51d6bda44f6ae18766ebe390118740bb9c7
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pengzhao-intel pengzhao-intel pengzhao-intel approved these changes

@szha szha Awaiting requested review from szha

Assignees
No one assigned
Labels
MKLDNN
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

FullyConnected op with float64 and MKL-DNN fails if gradient are not set in a specific way
2 participants
@ZhennanQin @pengzhao-intel