Skip to content

(closes #3262) Update lfric gpu script and collect aggregated transformation stats#3450

Open
sergisiso wants to merge 7 commits into
masterfrom
update_lfric_gpu_script
Open

(closes #3262) Update lfric gpu script and collect aggregated transformation stats#3450
sergisiso wants to merge 7 commits into
masterfrom
update_lfric_gpu_script

Conversation

@sergisiso

Copy link
Copy Markdown
Collaborator

No description provided.

@codecov

codecov Bot commented Jun 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (0194bcb) to head (59ef242).

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #3450   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          393       393           
  Lines        55065     55065           
=========================================
  Hits         55065     55065           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sergisiso

Copy link
Copy Markdown
Collaborator Author

@arporter @LonelyCat124 This is ready for review, its a small modification to the gpu offloading script but also adds the aggregated stats that I have been reporting. The ITs now show:
image

Should we also add integration checks for the numbers, eg "if number of module-inlined successes is < 277: fail"?

@sergisiso sergisiso self-assigned this Jun 2, 2026
@LonelyCat124

Copy link
Copy Markdown
Collaborator

I think having checks that we don't make this do worse is probably a good idea, if we have a good reason to make things worse then we can manually make that change as/when it happens.

@LonelyCat124

Copy link
Copy Markdown
Collaborator

I think having checks that we don't make this do worse is probably a good idea, if we have a good reason to make things worse then we can manually make that change as/when it happens.

I thought about this more - having an option to see which expected files are missed would be helpful for future PRs that cause issues.

@sergisiso

Copy link
Copy Markdown
Collaborator Author

@arporter @LonelyCat124 For now I added the suggested threshold checks. Listing which kernels diverged is harder because it requires keeping a full log of a working run, and this are not very stable: parallel runs, reflect any change, ... so for now I haven't done this second step but I maybe it can be a follow on PR.

@LonelyCat124

Copy link
Copy Markdown
Collaborator

@arporter @LonelyCat124 For now I added the suggested threshold checks. Listing which kernels diverged is harder because it requires keeping a full log of a working run, and this are not very stable: parallel runs, reflect any change, ... so for now I haven't done this second step but I maybe it can be a follow on PR.

@sergisiso Yeah thats reasonable - I think probably a script later (that isn't stored/run automatically) to let this be computed manually is good enough, but that doesn't need to be in this PR I think.

@LonelyCat124 LonelyCat124 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sergisiso This is good I think - the only thing I wonder is if we can have something explicit to state when we increase the specific ones we care about, e.g. in check_above

if [[$value -gt $2]]; then
   echo
   echo "Improvement: Number of $1 has increased to $value"
 fi

so when we want to review changes its spelled out without having to search the values so much. It is a bit unnecessary since the information is output anyway, but I'll leave it up to you - if you don't see the value I'll proceed to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants