Fix metric attribute lookup #8181
Conversation
ethanwharris
requested review from
ananyahjha93,
awaelchli,
justusschock,
SkafteNicki,
tchaton and
williamFalcon
as code owners
Codecov Report
@@ Coverage Diff @@
## master #8181 +/- ##
=======================================
- Coverage 93% 92% -0%
=======================================
Files 211 212 +1
Lines 13450 13810 +360
=======================================
+ Hits 12486 12766 +280
- Misses 964 1044 +80 |
ethanwharris
requested review from
Borda,
carmocca,
kaushikb11 and
SeanNaren
as code owners
| self.a_metric = SumMetric() | ||
|
|
||
| def training_step(self, *args): | ||
| metric = SumMetric() | ||
| self.log("foo", metric) | ||
|
|
||
| trainer = Trainer(default_root_dir=tmpdir, fast_dev_run=1) | ||
| model = TestModel() | ||
| with pytest.raises(MisconfigurationException, match=escape("where `name` is one of ['a_metric']")): | ||
| trainer.fit(model) |
There was a problem hiding this comment.
@carmocca why do we need this dependency on the attribute name in the module vs the key name used for publishing? if it's solely for restoration, could we think of other approaches? this style of metrics logging is a very common pattern for us
There was a problem hiding this comment.
Not @carmocca, but I'll take a shot at this - I don't think there is a dependency on key name for publishing. This test is failing because the metric being logged isn't the same instance as the self.a_metric metric that's an attribute in the module.
test_log_metric_dict below logs some metrics that have key name != attribute name and still passes
There was a problem hiding this comment.
@mooey5775 is correct.
This is to match the logged metric with the metric attribute in the LightningModule so the state can be restored.
could we think of other approaches?
Open to ideas!
|
Let's say I have some metrics which I don't want to restore/track (and therefore, this metric attribute is no needed). What's the recommended way to log these metrics in that case? |
But it's still a stateful metric? Can't change to a functional counterpart?
There isn't currently for this as the |
What does this PR do?
Fixes #8179
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃