Cache initializers to avoid iterating over them on every forward#7905
Merged
baijumeswani merged 1 commit intomasterfrom Jun 2, 2021
Merged
Cache initializers to avoid iterating over them on every forward#7905baijumeswani merged 1 commit intomasterfrom
baijumeswani merged 1 commit intomasterfrom
Conversation
baijumeswani
requested review from
BowenBao,
liqunfu,
mrry,
spandantiwari and
thiagocrepaldi
as code owners
mrry
approved these changes
Jun 1, 2021
Contributor
mrry
left a comment
mrry
left a comment
There was a problem hiding this comment.
Very nice! I'd be interested to see if we could push this optimization further into the TrainingAgent (and similarly for inference if we switch it to an API more like TrainingAgent), and avoid all the dlpack wrapping etc. on each call.
Contributor
|
+1 - great stuff to pick up for 1p/3p model even as is In reply to: 673591206 |
thiagocrepaldi
approved these changes
Jun 2, 2021
harshithapv
pushed a commit
that referenced
this pull request
Jun 16, 2021
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description: Cache initializers at the time of initializing the graph to avoid iterating over them on each forward call.
Also, this pull request removes one round of iteration over all the initializers to check the device matches with the model device. This check is redundant as there is already a check at the beginning of the forward method that does the same check and the graph execution would not move the initializers to another device during execution.