Skip to content

[1277] - Add remaining Laghos problems to benchpark#1278

Merged
pearce8 merged 13 commits into
developfrom
1277-add-all-laghos-problems-to-benchpark
Apr 15, 2026
Merged

[1277] - Add remaining Laghos problems to benchpark#1278
pearce8 merged 13 commits into
developfrom
1277-add-all-laghos-problems-to-benchpark

Conversation

@amroakmal

@amroakmal amroakmal commented Mar 4, 2026

Copy link
Copy Markdown
Collaborator

Resolves #1277

@amroakmal amroakmal self-assigned this Mar 4, 2026
@github-actions github-actions Bot added the experiment New or modified experiment label Mar 4, 2026
@amroakmal amroakmal force-pushed the 1277-add-all-laghos-problems-to-benchpark branch 4 times, most recently from 267af62 to d3dbf15 Compare March 5, 2026 22:05
@amroakmal amroakmal force-pushed the 1277-add-all-laghos-problems-to-benchpark branch 2 times, most recently from bfe7258 to 6207c3c Compare March 6, 2026 20:00
@amroakmal amroakmal force-pushed the 1277-add-all-laghos-problems-to-benchpark branch from 6207c3c to e24d7d9 Compare March 7, 2026 00:06
@amroakmal amroakmal added the ready for review Ready for review label Mar 8, 2026
@amroakmal

amroakmal commented Mar 8, 2026

Copy link
Copy Markdown
Collaborator Author

Update: I confirm testing the two remaining problems and they both finished successfully. I tested with a backup branch forked from this feature branch and this forked branch has been rebased on hip-wrapper branch. But this feature branch here does not have hip-wrapper feature branch logic/changes as that should be handled in its own PR.

=====

Update: Rebased on hip-wrapper, still facing same issue:

laghos: error while loading shared libraries: libunwind.so.1: cannot open shared object file: No such file or directory
flux-job: task(s) exited with exit code 127

=====

This has been tested for 6/8 different Laghos problems with different configs and params from what we added. It's ready for review, but I'll make sure its not merged until the remaining two problems are tested, blocked by the hip-wrapper PR.

@michaelmckinsey1 michaelmckinsey1 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.

Since this is working for everything but rocm/7, I would say this is not blocked by the hip-wrapper branch. Because we can still use it on the other architectures and rocm6.

So once this PR is ready, I think we can merge it independent of #1266. Also @scheibelp said 1266 won't likely get merged anyway, it will be fixed in an upstream spack PR (#49673), which will take longer than the benchpark PR to merge.

Comment thread experiments/laghos/experiment.py
Comment thread repos/ramble_applications/laghos/application.py Outdated
Comment thread experiments/laghos/experiment.py
Comment thread experiments/laghos/experiment.py Outdated
Comment thread experiments/laghos/experiment.py Outdated
Comment thread experiments/laghos/experiment.py Outdated
Comment thread repos/ramble_applications/laghos/application.py Outdated
Comment thread experiments/laghos/experiment.py
Comment thread experiments/laghos/experiment.py Outdated
Comment thread repos/ramble_applications/laghos/application.py Outdated
@amroakmal amroakmal force-pushed the 1277-add-all-laghos-problems-to-benchpark branch 6 times, most recently from f2cdf80 to 66201fc Compare March 18, 2026 01:31
@pearce8 pearce8 added changes requested Changes requested and removed ready for review Ready for review labels Mar 18, 2026
@amroakmal amroakmal force-pushed the 1277-add-all-laghos-problems-to-benchpark branch from 0f151af to 4cd8aa9 Compare March 18, 2026 03:16
@amroakmal amroakmal added ready for review Ready for review and removed changes requested Changes requested labels Mar 19, 2026
Comment thread experiments/laghos/experiment.py Outdated
@pearce8

pearce8 commented Mar 24, 2026

Copy link
Copy Markdown
Collaborator

@michaelmckinsey1 please review and let me know when this is ready for my review.

@michaelmckinsey1 michaelmckinsey1 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.

application.py looks fine now that we have common_args, much more succinct.

Comment thread experiments/laghos/experiment.py Outdated
Comment thread experiments/laghos/experiment.py
Comment thread experiments/laghos/experiment.py Outdated
@pearce8 pearce8 added the changes requested Changes requested label Mar 25, 2026
@pearce8

pearce8 commented Mar 25, 2026

Copy link
Copy Markdown
Collaborator

@michaelmckinsey1 please re-review, and remove the "changes requested" label when you resolve all requests and approve the PR.

@amroakmal amroakmal removed the changes requested Changes requested label Apr 8, 2026
@amroakmal amroakmal force-pushed the 1277-add-all-laghos-problems-to-benchpark branch from 0f7af3e to a405c90 Compare April 8, 2026 05:28

@michaelmckinsey1 michaelmckinsey1 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.

Looks good, given the Laghos gitlab tests that are running are successful

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.04%. Comparing base (415b563) to head (375d0bc).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1278      +/-   ##
===========================================
+ Coverage    62.48%   64.04%   +1.56%     
===========================================
  Files           49       49              
  Lines         3774     3774              
  Branches       326      326              
===========================================
+ Hits          2358     2417      +59     
+ Misses        1402     1343      -59     
  Partials        14       14              

see 2 files with indirect coverage changes

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

@pearce8 pearce8 merged commit 9bf10af into develop Apr 15, 2026
21 checks passed
@pearce8 pearce8 deleted the 1277-add-all-laghos-problems-to-benchpark branch April 15, 2026 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

experiment New or modified experiment ready for review Ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add remaining Laghos experiments to Benchpark with any parameters

4 participants