Repository navigation
Label time_data.dat and io_time_data.dat columns by what they measure - #1949
Draft
sbryngelson wants to merge 1 commit into
Draft
sbryngelson wants to merge 1 commit into
sbryngelson wants to merge 1 commit into
Conversation
time_data.dat's "s/step" column holds the fastest single RK stage (one RHS evaluation, ~1/3 of an RK3 step), and io_time_data.dat's holds the mean time of one save. Relabel them "s/rhs" and "s/save". Columns and values are unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The
time_data.datheader calls its second columns/step, but the value is the fastest single RK stage, i.e. one RHS evaluation. For RK3 that is about a third of a step, so anyone reading it as a per-step time underestimates cost by about 3x. Theio_time_data.datcolumn is also labelleds/step, but it holds the mean time of ones_save_datacall.Root cause. Since #1650,
s_tvd_rksetstime_avgto the minimum wall-clock time over individual RK stages, which is the right input for the grind time (ns/gp/eq/rhs). The header written ins_save_performance_metricswas never updated.Fix. The headers become
s/rhsands/save, and each gets a one-line comment. Columns, values and formats are unchanged. The only parser in the tree (helpers.mako, which reads the last column of the last line) is unaffected. An existingtime_data.datkeeps its old header, because rows are appended.Verification
I ran a 1-rank run of a small 2D case on a Frontier CPU compute node (GNU 12.3):
simulationcompiles with CCE 19 (CPU) and GNU 12.3. Precheck passes, apart from twotest_thermochemcases that fail on the Frontier login node because they compile with the system/usr/bin/gfortran(addressed by #1943). Results do not change.Contribution Policy
We do not accept pull requests generated primarily by AI without genuine understanding or real-world usage context.
All contributions are expected to demonstrate:
If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.
Acknowledgement
This PR was prepared with the assistance of an AI tool (Claude Code). We hit this while reading timings from production runs on Frontier.
PR template credit: junegunn