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

Executable to rule them all #1075

Merged
merged 8 commits into from
May 3, 2024
Merged

Conversation

danholdaway
Copy link
Contributor

Changes e.g. fv3jedi_variational.x config.yaml -> gdas_fv3jedi.x variational config.yaml. Do we like this?

Pros:

  • Build only executables we need.
  • Is this better NCO compliance?
  • Don't waste time building tests for fv3-jedi and soca.

Cons:

  • Maintenance increase.
  • Needs G-W changes.

@danholdaway
Copy link
Contributor Author

Building wont work in the tests because the executables can't be made

@aerorahul
Copy link
Contributor

Why not go one step further and have:

gdas_jedi.x fv3jedi variational config.yaml

or

gdas_jedi.x soca hofx config.yaml

One executable to rule them all!

@guillaumevernieres
Copy link
Contributor

I like it 👍 . That would also give us the flexibility to add some of the pre-and post processing tasks within the same executable too.

Copy link
Contributor

@CoryMartin-NOAA CoryMartin-NOAA left a comment

Choose a reason for hiding this comment

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

Fantastic! I think this is relatively little work for a lot of simplification in terms of workflow, linking executables, and frankly, telling people how to use JEDI.;

@guillaumevernieres
Copy link
Contributor

FYI @danholdaway , this could be tested with soca without a PR in the g-w. We haven't updated our agricultural script that runs the variational application yet.

@danholdaway
Copy link
Contributor Author

danholdaway commented Apr 26, 2024

Why not go one step further and have:

gdas_jedi.x fv3jedi variational config.yaml

or

gdas_jedi.x soca hofx config.yaml

One executable to rule them all!

I agree that that would be quite nice. Is that still NCO compliant? The line "divide the source files into sub-directories according to the executable they produce" makes it sound like they want separate executables for the gw/sorc/gdas/sorc/code directories. Let me also check that it doesn't make for very massive executables.

@CoryMartin-NOAA
Copy link
Contributor

No I think the most NCO compliant solution as I interpret it is one giant gdas.x

@danholdaway
Copy link
Contributor Author

No I think the most NCO compliant solution as I interpret it is one giant gdas.x

Ok great, I'll try that out. By the way when I tested this I was expecting to find only gdas_code.x in the bin directory but the cmake on JEDI bundle fills the /bin directory with Python "executables". Something perhaps to keep in mind since right now we can't stop that from happening and that might cause some consternation from NCO.

@aerorahul
Copy link
Contributor

No I think the most NCO compliant solution as I interpret it is one giant gdas.x

Ok great, I'll try that out. By the way when I tested this I was expecting to find only gdas_code.x in the bin directory but the cmake on JEDI bundle fills the /bin directory with Python "executables". Something perhaps to keep in mind since right now we can't stop that from happening and that might cause some consternation from NCO.

I would suggest we work on make install capability and would only install whats built. Additionally, only required executables and python codes should be built. Python code is "technically" not a compiled executable, however, it is being "configured" at build time. We will need to think about what the "install" target for this is pre-code handoff.

@CoryMartin-NOAA
Copy link
Contributor

@danholdaway I fully expect to not even include the python iodaconv repo when this gets implemented. I suspect we will only be using compiled code for conversion then.

@aerorahul
Copy link
Contributor

Why not go one step further and have:

gdas_jedi.x fv3jedi variational config.yaml

or

gdas_jedi.x soca hofx config.yaml

One executable to rule them all!

I agree that that would be quite nice. Is that still NCO compliant? The line "divide the source files into sub-directories according to the executable they produce" makes it sound like they want separate executables for the gw/sorc/gdas/sorc/code directories. Let me also check that it doesn't make for very massive executables.

In this case, we will be linking one gdas_jedi.cd to one gdas_jedi.x as opposed to the numerous ones for each application (e.g. fv3jedi_variational.x, soca_hofx.x, etc.). In that way, it is no different, but provides a single entry point to all functions. It also is in the same vein as running an executable with a different configuration provided by a namelist.

@danholdaway
Copy link
Contributor Author

g-w PR NOAA-EMC/global-workflow#2565

@danholdaway danholdaway marked this pull request as ready for review May 2, 2024 19:02
@danholdaway
Copy link
Contributor Author

Ready for final review along with global workflow PR

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 @danholdaway

* develop:
  Using ioda util to convert the datetime in AMSR2 converter (#1077)
  Add modulefile for Dogwood/Cactus (#1073)
  Addition of a switch for the cycling type (#1072)
Copy link
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

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

this is excellent!

@CoryMartin-NOAA CoryMartin-NOAA merged commit 70f1319 into develop May 3, 2024
5 checks passed
@CoryMartin-NOAA CoryMartin-NOAA deleted the feature/build_jedi_exe branch May 3, 2024 18:42
@danholdaway
Copy link
Contributor Author

Thanks all. What a day, my first contribution to GDASapp 🥹

WalterKolczynski-NOAA pushed a commit to NOAA-EMC/global-workflow that referenced this pull request May 6, 2024
Changes that accompany GDAS PR (NOAA-EMC/GDASApp/pull/1075) that allows
building of a single gdas executable, which should be more compliant
with NCO requirements.

Addresses NOAA-EMC/GDASApp#1085
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.

None yet

4 participants