-
Notifications
You must be signed in to change notification settings - Fork 841
fix(model): resolve update-called-attribute issue #2862 #2873
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
base: main
Are you sure you want to change the base?
fix(model): resolve update-called-attribute issue #2862 #2873
Conversation
|
Hello @harsh1504660 , all good ? |
|
Hi @psous32, |
|
Thank you @harsh1504660 , I have to thank you for keeping this lib with a good support |
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.
Hi @harsh1504660
Thanks for the attempt to fix the error.
I have some comments:
- I can not reproduce the same error about the update_called not being there.
As far as I can see, update_called is a standard property that should exist on all metrics from since torchmtrics 1.0 release.
Source: https://github.com/Lightning-AI/torchmetrics/blob/3a26afb551de6a2f479194c77e8b8bc8a6cd3f53/CHANGELOG.md?plain=1#L626
https://github.com/Lightning-AI/torchmetrics/blob/3a26afb551de6a2f479194c77e8b8bc8a6cd3f53/src/torchmetrics/metric.py#L187
torchmetrics version: 1.7.4
hasattr(metric, "update_called"): True
Which version of torch metrics are you using? Please note that the anomalib requires torchmetrics>1.3
- It might be nice to not use the private _update_called as it has been long deprecated.
| @@ -135,15 +135,15 @@ def on_validation_epoch_end(self, trainer: Trainer, pl_module: LightningModule) | |||
| del trainer, pl_module | |||
| if self.enable_thresholding: | |||
| # compute threshold values | |||
| if self._image_threshold_metric.update_called: | |||
| if getattr(self._image_threshold_metric, "_update_called", False): | |||
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.
I am personally not in favour of using getattrs if possible. If the image threshold metric isn't set, it will be None. In that case we can just compare it against null value.
📝 Description
The change uses getattr(...) to safely check if _update_called exists before accessing it, ensuring compatibility with other models and avoiding unexpected crashes.
✨ Changes
Select what type of change your PR is:
✅ Checklist
Before you submit your pull request, please make sure you have completed the following steps:
For more information about code review checklists, see the Code Review Checklist.