Skip to content

Conversation

@yufenglee
Copy link
Member

Description: Describe your changes.
Prepacking uses the packed tensor for computation, which prevents the weight update on CPU.

@yufenglee yufenglee requested a review from a team as a code owner March 31, 2021 20:48

if (disable_prepacking != "1") {
ORT_RETURN_IF_ERROR(PrepackConstantInitializedTensors(constant_initializers_use_count));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems disable prepacking is off by default? If I read the code correctly, the change here just removed disable prepacking option, instead of changing the default value?

Copy link
Member Author

Choose a reason for hiding this comment

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

PrepackConstantInitializedTensors is the function to prepack the tensor

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe some comments why prepack is disabled for training here?

@ke1337
Copy link
Contributor

ke1337 commented Mar 31, 2021

Could we add a test case for CPU training with Gemm to make sure no regression in future?

@yufenglee
Copy link
Member Author

Could we add a test case for CPU training with Gemm to make sure no regression in future?

Agree. We need to add unit tests in the training branch to cover enough scenarios.

@yufenglee yufenglee merged commit 34a8b22 into master Apr 1, 2021
@yufenglee yufenglee deleted the yufeng/pre_pack_training branch April 1, 2021 21:03
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.

4 participants