Skip to content

Add refinediag preanalysis tests#229

Merged
singhd789 merged 73 commits into
NOAA-GFDL:mainfrom
singhd789:216.refinediag-preanalysis-tests
May 21, 2026
Merged

Add refinediag preanalysis tests#229
singhd789 merged 73 commits into
NOAA-GFDL:mainfrom
singhd789:216.refinediag-preanalysis-tests

Conversation

@singhd789
Copy link
Copy Markdown
Contributor

@singhd789 singhd789 commented Apr 13, 2026

Describe your changes

  • added dummy preanalysis and refinediag scripts
  • added preanalysis and refinediag settings in yaml config (for both local and remote)
  • updated test_cloud_runner pipeline for tasks that needed to be checked
  • added sed line in runscript to check a line was changed (not really needed though, more for my own sanity)
  • updated flow.cylc for preanalysis and refinediag tasks (missing some lines/needed to be updated)

Issue ticket number and link (if applicable)

Fixes #216

Checklist before requesting a review

  • I ran my code
  • I tried to make my code readable
  • I tried to comment my code
  • I wrote a new test, if applicable
  • I wrote new instructions/documentation, if applicable
  • I ran pytest and inspected it's output
  • I ran pylint and attempted to implement some of it's feedback
  • No print statements; all user-facing info uses logging module

Manual Pipeline Run Details

Was the manual pipeline (test_cloud_runner) triggered for this PR?

(Paste relevant logs, output, or a link to the workflow run here)

How to trigger the manual pipeline:

The test_cloud_runner pipeline is not automatically associated as a required check with the PR; it must be triggered to test changes in a full post-processing run.

To trigger the manual pipeline:

  1. Follow the link to the test_cloud_runner actions tab here

    • you should see "This workflow has a workflow_dispatch event trigger"
  2. Click the dropdown "Run workflow":

    a. If trying to merge from a branch on fre-workflows: choose branch from the first drop down, leave the next 2 inputs blank, and choose the fre-cli branch to test

    b. If trying to merge from a fre-workflows fork: can skip first branch selection, input the fork name (ex: [user]/fre-workflows), input the fork's branch name, and choose the fre-cli branch to test

  3. Click "Run workflow"

Note: you may need to reload the page to see your running workflow.

Dana Singh and others added 30 commits March 30, 2026 16:27
…ows into 216.refinediag-preanalysis-tests
…sis-tests"

This reverts commit 0efbaab, reversing
changes made to b12d98b.
@singhd789 singhd789 changed the title Add refinediag preanalysis remote tests Add refinediag preanalysis tests Apr 29, 2026
@singhd789 singhd789 marked this pull request as ready for review April 29, 2026 20:59
# to have a faster turn-around (does not test archive data staging), use the below
history_dir: "/work/inl/test_cloud_runner_history_files"
history_dir: "/work/inl/test_cloud_runner_history_files"
refined_history_dir: "/work/d4s/fre-wf-local-testing/refined_history"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about /work/$USER/... instead so it will work for everyone?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OH, good catch! I meant to change this

Copy link
Copy Markdown
Contributor Author

@singhd789 singhd789 May 6, 2026

Choose a reason for hiding this comment

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

Chris....you may have just really helped the problem I've had for like 2 weeks with this catch. (at least for the local test). I didn't realize the refineDiag post-script was looking somewhere else (with $USER) for files.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Wait...uhg, nevermind, I see my same issue again. It seems when I update the refine_history path to something new, the refineDIag post-script works. However, if I don't update the path and run again, it does not somehow. Continuing to look into

Dana Singh and others added 10 commits May 6, 2026 11:30
…ows into 216.refinediag-preanalysis-tests
…hub.com:singhd789/fre-workflows into 216.refinediag-preanalysis-tests
- uhg - we were cleaning the experiment but not this output folder which lead to a known issue with hsmget (can't have folders wrapped in a tar file)
Comment thread for_gh_runner/runscript.sh Outdated
# this might be better in the DOckerfile?
#print the line to make sure it was changed in the build
sed -n '28p' /app/cylc-flow-tools/mk/hsmput.mk

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lying in bed last night, I thought that I should have suggested removing hsmget and hsmput entirely at the first glimpse that they were non-portable, and replaced them with humble tars.

But.. since we're this far, can we use hsm/1.4.0 instead of this sed?

Copy link
Copy Markdown
Contributor Author

@singhd789 singhd789 May 21, 2026

Choose a reason for hiding this comment

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

Yeah, luckily this line I think is only checking that that line was changed with the -n option - so it's not actually changing anything here. I can remove this line.

Now, unfortunately, in order to use the hsm1.4.0 package, I had to update the cylc-flow-tools.yaml , which meant the container needed to be rebuilt.....but when I built it again is when I realized fregrid was not available anymore 😢

Comment thread site/ppan.cylc
Comment thread site/ppan.cylc Outdated
Copy link
Copy Markdown
Contributor

@ceblanton ceblanton left a comment

Choose a reason for hiding this comment

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

Except for the ppan.cylc test changes, this is more than ready.

How are we going to get fre-nctools back into the cloud pipeline?

Copy link
Copy Markdown
Contributor

@ceblanton ceblanton left a comment

Choose a reason for hiding this comment

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

It looks perfect. Thank you so much for the this, which turned out to be so much harder than I could have imagined.

@singhd789
Copy link
Copy Markdown
Contributor Author

It looks perfect. Thank you so much for the this, which turned out to be so much harder than I could have imagined.

I just don't know how it was working before 😆

@singhd789 singhd789 merged commit 7ec9437 into NOAA-GFDL:main May 21, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add test case for refineDiag and preAnalysis capability in workflow

2 participants