Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experiment/salmon #619

Merged
merged 20 commits into from
Mar 25, 2025
Merged

Experiment/salmon #619

merged 20 commits into from
Mar 25, 2025

Conversation

slabasan
Copy link
Collaborator

@slabasan slabasan commented Feb 14, 2025

Depends on #703 to pass lint CI checks.

Blocked by:

  • Update version of Spack #623 -- debug errors when updating spack version Remove depends_on() and no longer need a newer spack
  • Babelstream package in benchpark vs updated in spack Fixed in develop

Description

Adding/modifying a benchmark (docs: Adding a Benchmark)

  • If modifying the source code of a benchmark: create, self-assign, and link here a follow up issue with a link to the PR in the benchmark repo.
  • If package.py upstreamed to Spack is insufficient, add/modify repo/benchmark_name/package.py plus: create, self-assign, and link here a follow up issue with a link to the PR in the Spack repo.
  • If application.py upstreamed to Ramble is insufficient, add/modify repo/benchmark_name/application.py plus: create, self-assign, and link here a follow up issue with a link to the PR in the Ramble repo.
  • Tags in Ramble's application.py or in repo/benchmark_name/application.py will appear in the docs catalogue
  • Add/modify an experiments/benchmark_name/experiment.py to define a single node and multi-node experiments
  • Add/modify a dry run unit test in .github/workflows/run.yml

@github-actions github-actions bot added experiment New or modified experiment ci CI, unit tests, GitHub actions application labels Feb 14, 2025
@pearce8 pearce8 mentioned this pull request Feb 18, 2025
9 tasks
@pearce8
Copy link
Collaborator

pearce8 commented Feb 18, 2025

@slabasan What was the reason to pull in a new version of Spack here? I think it is causing some of the issues with your tests, so I pulled out the version update into its own PR #623 to try to figure that part out.

@slabasan slabasan added the WIP A work-in-progress not yet ready to commit label Feb 18, 2025
pearce8 pushed a commit that referenced this pull request Feb 24, 2025
* Remove legacy experiment, modifier, and system checks

* Migrate legacy CI to use current workflow

* Omit experiment that has not been migrated yet #619

* Fix path for dynamic dry run

* Fix saxpy/openmp test

* Initialize fugaku system in qws/openmp

* Fix bug by changing benchmark to system

* Remove legacy modifier config

* Unlink legacy modifier

---------

Co-authored-by: Mckinsey <[email protected]>
Co-authored-by: Michael Richard Mckinsey <[email protected]>
@slabasan slabasan force-pushed the experiment/salmon branch 7 times, most recently from 2323bfc to 028b6c3 Compare March 24, 2025 23:29
@slabasan slabasan force-pushed the experiment/salmon branch from 028b6c3 to 98420cf Compare March 25, 2025 00:54
@slabasan
Copy link
Collaborator Author

slabasan commented Mar 25, 2025

The lint CI check caught a mis-spelling of optimization in repo/salmon_tddft/fjmpi.patch. But is also thinking that nd is a mis-spelled 2nd. This is a false positive error. Should lint CI be checking these files?

Update: I've updated our github actions to ignore applying codespell to patch files. These files seem out of scope for benchpark.

Copy link
Collaborator

@michaelmckinsey1 michaelmckinsey1 left a comment

Choose a reason for hiding this comment

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

Looks good once lint passing

@slabasan slabasan force-pushed the experiment/salmon branch from db2ba9b to 3bd93a2 Compare March 25, 2025 20:43
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Mar 25, 2025
@slabasan
Copy link
Collaborator Author

This one is (finally) ready!

@slabasan slabasan requested a review from dyokelson March 25, 2025 20:46
Copy link
Collaborator

@dyokelson dyokelson left a comment

Choose a reason for hiding this comment

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

Do we want all the tests in the run.yml or just in the new format? Just not sure if lines 265-285 are still needed or it would be best to just keep the later versions.

We should have the Riken folks test this out on Fugaku as well.

@pearce8 pearce8 added this pull request to the merge queue Mar 25, 2025
Merged via the queue into develop with commit 43df7c2 Mar 25, 2025
11 checks passed
@pearce8 pearce8 deleted the experiment/salmon branch March 25, 2025 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
application ci CI, unit tests, GitHub actions dependencies Pull requests that update a dependency file experiment New or modified experiment WIP A work-in-progress not yet ready to commit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants