Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces rolling window goodput monitoring to MaxText. It adds a new context manager maybe_monitor_rolling_window_goodput in goodput.py and exposes corresponding configuration options (rolling_windows_seconds and enable_rolling_window_goodput) in base.yml and types.py. The feedback recommends tracking whether the rolling window uploader successfully started before stopping it in the finally block to prevent masking initialization exceptions, and suggests using list[PositiveInt] in the Pydantic configuration to enforce positive integer intervals.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
f291ce5 to
152bcb3
Compare
cbbb942 to
5066bce
Compare
5066bce to
dc0ef22
Compare
|
Re: "Track Test Duration" failure. This isn't caused by this PR. The failing check only compares TPU unit-test durations against the last scheduled That includes This PR only touches
Bold = flagged (more than 20% and more than 15s over the baseline). Each scheduled |
dc0ef22 to
6579208
Compare
|
Overall, LGTM! |
dipannita08
left a comment
There was a problem hiding this comment.
Left a couple small comments, LGTM otherwise!
b78e0db to
3e49e6b
Compare
…w for more accurate measurement of performance and help identify trends in data throughput.
3e49e6b to
a4f335e
Compare
Description
Enables rolling window Goodput tracking in MaxText to monitor performance trends over configurable intervals (1h, 1d, 3d, 5d). This feature allows for identifying performance degradation or improvement trends within specific time windows, supplementing the cumulative goodput metrics.
BUGS: b/556291470
Tests
Verified with test runs remotely on TPU VMs. Confirmed metrics are generated and sent to Google Cloud Monitoring for all configured windows.
Command:
Checklist
Reference:
Before submitting this PR, please make sure (put X in square brackets):
gemini-reviewlabel.