Skip to content
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

Refine INC shutdown code #335

Merged
merged 8 commits into from
Sep 25, 2024
Merged

Conversation

kzawora-intel
Copy link

This PR removes debug printouts in INC shutdown method and covers the case where application exits before model is initialized properly.

Comment on lines 1943 to 1958
from operator import attrgetter
can_finalize_inc = False
try:
is_inc_used = attrgetter("model_config.quantization")(
self) == 'inc'
is_model_initialized = attrgetter("model.model")(self) is not None
is_inc_initialized = attrgetter("inc_initialized_successfully")(
self)
can_finalize_inc = is_inc_used and is_model_initialized and \
is_inc_initialized
except AttributeError:
pass
if can_finalize_inc:
from neural_compressor.torch.quantization import (
finalize_calibration)
finalize_calibration(self.model.model)

Choose a reason for hiding this comment

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

Oh, I thought attrgetter can handle AttributeErrors ...

So what's the point of using it at the moment? Since we're catching the exception, couldn't we do:

        try:
            is_inc_used = self.model_config.quantization == 'inc'
            is_model_initialized = self.model.model is not None
            is_inc_initialized = self.inc_initialized_successfully
            can_finalize_inc = is_inc_used and is_model_initialized and \
                is_inc_initialized
        except AttributeError:
            pass

or even better:

 can_finalize_inc = (self.model_config.quantization == 'inc') and \
    (self.model.model is not None) and \
    self.inc_initialized_succesfully

Copy link
Author

Choose a reason for hiding this comment

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

fixed + added protection against double shutdown

Copy link

@madamczykhabana madamczykhabana left a comment

Choose a reason for hiding this comment

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

LGTM

@michalkuligowski michalkuligowski merged commit 8c6dcae into habana_main Sep 25, 2024
19 checks passed
@kzawora-intel kzawora-intel added the habana Issues or PRs submitted by Habana Labs label Sep 30, 2024
@kzawora-intel kzawora-intel deleted the private/kzawora/inc_shutdown branch October 7, 2024 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
habana Issues or PRs submitted by Habana Labs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants