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

Make separate JCB algorithm YAMLs for CI tests #1394

Merged
merged 6 commits into from
Dec 2, 2024
Merged

Conversation

DavidNew-NOAA
Copy link
Collaborator

The motivation for having separate JCB algorithm YAMLs for each CI test is that in the future, we may want different regression tests for various resolutions, obs configurations, etc. The primary JCB algorithm YAMLs should be generic.

danholdaway
danholdaway previously approved these changes Dec 2, 2024
Copy link
Contributor

@danholdaway danholdaway left a comment

Choose a reason for hiding this comment

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

Do you want to further your if statement around the testing to also include resolution? Especially since that's hardcoded in the path for the reference files. And/or template the reference file names to include the resolution?

@DavidNew-NOAA
Copy link
Collaborator Author

@danholdaway Interesting idea. Let me give that a try

Copy link
Contributor

@guillaumevernieres guillaumevernieres left a comment

Choose a reason for hiding this comment

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

Thanks @DavidNew-NOAA . That should fix the issue that we have when testing the 3dfgat in static and hybrid mode, it currently points to the same ref file.

@DavidNew-NOAA
Copy link
Collaborator Author

@danholdaway OK, I implemented your suggestion

Copy link
Contributor

@guillaumevernieres guillaumevernieres left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@guillaumevernieres guillaumevernieres left a comment

Choose a reason for hiding this comment

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

👍

@DavidNew-NOAA DavidNew-NOAA merged commit 4f50d78 into develop Dec 2, 2024
5 checks passed
@DavidNew-NOAA DavidNew-NOAA deleted the feature/gw-ci branch December 2, 2024 19:42
@guillaumevernieres
Copy link
Contributor

I did approve, but that was with the understanding that we should be running the gdasapp test @DavidNew-NOAA . Are these broken? I see that the label I set in my pr has not been updated.

@DavidNew-NOAA
Copy link
Collaborator Author

@guillaumevernieres I'm actually refactoring and testing those scripts right now, so they won't work right now with the CI labels. I wasn't aware anyone was using those scripts currently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants