Skip to content

(closes #3243) Unify NEMO parallelisation scripts#3443

Open
sergisiso wants to merge 59 commits into
masterfrom
single_nemo_script_v2
Open

(closes #3243) Unify NEMO parallelisation scripts#3443
sergisiso wants to merge 59 commits into
masterfrom
single_nemo_script_v2

Conversation

@sergisiso
Copy link
Copy Markdown
Collaborator

@sergisiso sergisiso commented May 26, 2026

This is the continuation of #3244 , I started a new PR because I shared the branch associated with the previous PR with multiple people that may be using them and modifying may break their workflows.

The PR aims to:

sergisiso and others added 30 commits October 15, 2025 10:10
@codecov
Copy link
Copy Markdown

codecov Bot commented May 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.96%. Comparing base (5f5004c) to head (07221fd).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3443   +/-   ##
=======================================
  Coverage   99.96%   99.96%           
=======================================
  Files         391      391           
  Lines       54689    54715   +26     
=======================================
+ Hits        54670    54696   +26     
  Misses         19       19           

☔ View full report in Codecov by Sentry.
📢 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

@LonelyCat124 This is ready for another look, we had a first review some time ago in #3244 and there was open comments there.

@sergisiso
Copy link
Copy Markdown
Collaborator Author

@LonelyCat124 Removing 9ee8eb6 has worked here, so this is ready to review again.

Copy link
Copy Markdown
Collaborator

@LonelyCat124 LonelyCat124 left a comment

Choose a reason for hiding this comment

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

@sergisiso I think this is almost ready - have a few comments about the new README to fix up.

Comment thread examples/nemo/scripts/README.md Outdated
> ```
> when applying the transformations to NEMOv4.

TODO: Mention `ASYNC_PARALLEL`, `ENABLE_INLINING`, `PROFILING`
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.

TODO - is this for this PR or for a future PR?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I want to follow up this with with a few more changes but the PR has already become larger than I wanted, so I created a new #3445 issue to expand this and address other improvements. I added the number in the TODO tag.

Comment thread examples/nemo/scripts/README.md Outdated
it is useful to apply the transformations step-by-step while checking if the
`run.stat` values change. Some useful steps are:

- Starting building NEMO *without* `psyclonefc` and conservative optimisation flags
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.

Initially build NEMO ...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

Comment thread examples/nemo/scripts/README.md Outdated

- Starting building NEMO *without* `psyclonefc` and conservative optimisation flags
and run it serially (O2, no vectorisation, no-fma). Then store the generated `run.stat`.
- Then switch to using `psyclonefc` with the `PSYCLONE_OPTS="-s passthrough.py"`,
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.

I'm not sure I like the repeated Then's here - could be better to use an enumerated list and remove the "Starting/Then/finall"?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

Comment thread examples/nemo/scripts/README.md Outdated
Alongside finding which step is causing the divergence we may want to find
which file/s are causing it. This folder also contains a `do_file_by_file.sh`
script that build NEMO many times, each with only one file being transformed,
and compares the results with the stores `run.stat`
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.

s/stores/stored

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done, but I also commented this paragraph and added a TODO to the follow up PR as I haven't added the do_file_by_file.sh script yet.

@sergisiso
Copy link
Copy Markdown
Collaborator Author

@LonelyCat124 This is ready for another review

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.

2 participants