Skip to content

Save module output for backward if needed#8010

Merged
SherlockNoMad merged 2 commits intomasterfrom
bahuang/save_for_backward
Jun 10, 2021
Merged

Save module output for backward if needed#8010
SherlockNoMad merged 2 commits intomasterfrom
bahuang/save_for_backward

Conversation

@SherlockNoMad
Copy link
Contributor

@SherlockNoMad SherlockNoMad commented Jun 9, 2021

If a module output is also used in backward graph, we add it into ctx.save_for_backward(), so that pytorch will be able to catch if there is any inplace corruption.

Here's a sample pytorch error for catch such corruption:

RuntimeError: one of the variables needed for gradient computation has been modified by an inplace operation: [torch.cuda.FloatTensor [32, 500]], which is output 0 of _ORTModuleFunctionBackward, is at version 1; expected version 0 instead. Hint: enable anomaly detection to find the operation that failed to compute its gradient, with torch.autograd.set_detect_anomaly(True).

@SherlockNoMad SherlockNoMad merged commit 2a74f5e into master Jun 10, 2021
@SherlockNoMad SherlockNoMad deleted the bahuang/save_for_backward branch June 10, 2021 16:56
harshithapv pushed a commit that referenced this pull request Jun 16, 2021
* Save module output for backward if needed
harshithapv added a commit that referenced this pull request Jun 18, 2021
* Cache initializers and avoid device check ot end of forward (#7905)

* ATenOp Enhancement (#7725)

* config parser, default argument values

* ut

* win build

* maxpool2d

* fix win build

* fix build

* unfold atenop

* Update CMakeLists.txt for openvino EP (#7980)

* Add SoftmaxCrossEntropyLossInternal to Support Dynamic ignore_index Input (#7899)

* add SoftmaxCrossEntropyLossInternal

* bugfix and ut

* fix ut

* fix ut

* support torch1.8.1

* function body for nll_loss_internal

* Override ORTModule named_modules to support extra arg (#7954)

* add missing provider_options.h in packages (#7995)

* consolidate copy binary script for gpu/trt tarball package

* add provider_options.h

* add provider_options.h

* Add cuda provides files (#8002)

* Save module output for backward if needed (#8010)

* Save module output for backward if needed

* Make logic in InsertCastTransformer around forcing a node to fp32 more precise. (#8018)

* Address #7981

Reworked the logic around forcing a node to run on fp32 even if it was supported on fp16.

The github issue had multiple factors. In ORT 1.8 we remove Identity nodes that produce graph outputs as they're not needed. That resulted in a Loop node no longer having output nodes (it produces graph outputs instead), which meant the check in IsSingleInputNodeFloat16Node returned true as there was no longer a downstream Identity node processing fp16 data.

We shouldn't only force a node to fp32 in very specific circumstances, and the changes hopefully check for those more precisely.

* Fix Memory Leak from DlpackToOrtValue (#8029)

* Update DirectML EP changes from DmlDev as of 2021-06-07 (#7987)

* Merged PR 6093117: Fix test_DynamicQuantizedLinear_max_adjusted_expanded by allowing Identity operator to run on non-float inputs

Motivation:
As part of the OnnxConformance Backend tests, DynamicQuantizedLinear_max_adjusted_expanded is failing.

Root Cause:
- The test model has `Identity` operator as one of the node. The input of this node is of non-float data type.
- In DML, `Identity` operator is registered as operator which requires floating input.
- As per `DirectMLSchema.h`, support for non-float input has been added for `Identity` operator in DML but the same has not been reflected in the `OperatorRegistration.cpp`.

Changes:
- Removed all traces of the requiresFloatFormatsForGraph flag from it's definition and usage. This flag was only used for Identity and it's related operator.
- Added null check for the graphOutput nodeArg in GraphDescBuilder.cpp to stop the crash of the test.

Related work items: #33076298

* Merged PR 6103324: Remove usage of non-generic error code (FWP_E_NULL_POINTER)

Motivation:
Addressing Dwayne comment on the previous PR. [Ref: [6093117](https://dev.azure.com/microsoft/WindowsAI/_git/onnxruntime/pullrequest/6093117?discussionId=44292162&path=%2Fonnxruntime%2Fcore%2Fproviders%2Fdml%2FDmlExecutionProvider%2Fsrc%2FGraphPartitioner.cpp)]

Changes:
Inside the DML EP, we should not use some other platform specific error codes. Instead we should a appropriate generic error code.

Related work items: #33076298

Co-authored-by: Sumit Agarwal <sumitagarwal@microsoft.com>

* [js/react_native] Use a mobile ORT instead of a full ORT (#8042)

* Change full ort to mobile ort

* Update Android example to load mobile ort

* Change the format of test models to ort

* update ios to use mobile ort

* revise README

* use onnxruntime-mobile-c CocoaPods in a npm package

* fix PATH addition in windows

should set PATH, not add to the tail the copy of PATH

* Reduce Kernel Optimization (#8067)

* reduce optimization

* bug fix

* add a check

* add ut

* refactor

* add ut cases for keepdims=true

Co-authored-by: baijumeswani <bmeswani@microsoft.com>
Co-authored-by: Vincent Wang <wangwchpku@outlook.com>
Co-authored-by: Changming Sun <chasun@microsoft.com>
Co-authored-by: George Wu <jywu@microsoft.com>
Co-authored-by: Ryan Hill <38674843+RyanUnderhill@users.noreply.github.com>
Co-authored-by: Sherlock <baihan.huang@gmail.com>
Co-authored-by: Scott McKay <skottmckay@gmail.com>
Co-authored-by: sumitsays <sumitagarwal330@gmail.com>
Co-authored-by: Sumit Agarwal <sumitagarwal@microsoft.com>
Co-authored-by: Sunghoon <35605090+hanbitmyths@users.noreply.github.com>
Co-authored-by: iperov <lepersorium@gmail.com>
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.

5 participants