Configurable metric formatting for RichProgressBar #18373
Conversation
quintenroets
requested review from
williamFalcon,
awaelchli,
carmocca,
justusschock,
edenlightning,
lantiga and
Borda
as code owners
…ture-18367-configurable-metric-formatting
awaelchli
added
feature
awaelchli
changed the title
| yield f"{k}: {round(v, 3) if isinstance(v, float) else v}" | ||
| for name, value in self._metrics.items(): | ||
| if not isinstance(value, str): | ||
| value = round(value, 3) if self._metrics_format is None else f"{value:{self._metrics_format}}" |
There was a problem hiding this comment.
metrics_format could have .3f as default which should be equivalent to round(, 3):
There was a problem hiding this comment.
I definitely agree. It would not entirely be backward compatible for metrics with lower precision. That's why I didn't propose it originally.
e.g.
100.1 becomes
- round(, 3) => 100.1
- .3f => 100.100
In my opinion, the .3f option is actually better than the current behavior because it results in consistent metrics lengths, making the progress bar values jump around less, and making the metrics more readable. (For example, if 10.111 changes to 10.1, this introduces a shift in all other metric values)
There was a problem hiding this comment.
Good point. I don't think we need to be this strict about backward compatibility in terms of how the data is presented on the progress bar. Especially if the change is it's for a better UI
There was a problem hiding this comment.
Okay, I agree. I created a PR for this at #18483
Sure, I added two tests at #18483 |
What does this PR do?
Fixes #18367
This PR makes the string formatting of metric values configurable.
This allows for tracking losses in non-standard scenarios (e.g losses in the range 1e-7).
Before submitting
PR review
Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:
Reviewer checklist