-
Notifications
You must be signed in to change notification settings - Fork 249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PT] change api for get_config and load_from_config #3359
[PT] change api for get_config and load_from_config #3359
Conversation
57f8c5e
to
e448ac0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Please address my minor comments.
README.md
Outdated
@@ -283,7 +284,7 @@ resuming_checkpoint = torch.load(path_to_checkpoint) | |||
nncf_config = resuming_checkpoint['nncf_config'] | |||
state_dict = resuming_checkpoint['state_dict'] | |||
|
|||
quantized_model = nncf.torch.load_from_config(model, nncf_config, example_input) | |||
quantized_model = load_from_config(model, nncf_config, example_input) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO: What do you think about such usage?
quantized_model = load_from_config(model, nncf_config, example_input) | |
quantized_model = nncf.torch.load_from_config(model, nncf_config, example_input) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It won't work
Backend-specific modules is not part of nncf namespace
import nncf
nncf.torch.load_from_config
# AttributeError: module 'nncf' has no attribute 'torch'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you fix this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not in this pr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about
import nncf.torch
nncf.torch.load_from_config
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will works, but i prefer
from nncf.torch import load_from_config
load_from_config
README.md
Outdated
@@ -271,7 +272,7 @@ quantized_model = nncf.quantize(model, calibration_dataset) | |||
# Save quantization modules and the quantized model parameters | |||
checkpoint = { | |||
'state_dict': model.state_dict(), | |||
'nncf_config': model.nncf.get_config(), | |||
'nncf_config': get_config(model), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO: What do you think about such usage?
'nncf_config': get_config(model), | |
'nncf_config': nncf.torch.get_config(model), |
Changes
Add
get_config
tonncf/torch/__init__.py
Use function
get_config
andload_from_config
for new and old tracing asfrom nncf.torch import get_config, load_from_config
Tests
https://github.com/openvinotoolkit/nncf/actions/runs/13974357105