search

esmf-org / esmf

The Earth System Modeling Framework (ESMF) is a suite of software tools for developing high-performance, multi-component Earth science modeling applications.
https://earthsystemmodeling.org/
Other
149 stars 70 forks source link

Enable PIO with mpiuni #205

Closed billsacks closed 2 weeks ago

billsacks commented 6 months ago

This PR enables building and running with the internal PIO when using mpiuni. This is especially relevant for people using ESMPy, since ESMPy is sometimes built without mpi – and this is apparently needed on many HPC systems (see also https://github.com/conda-forge/esmpy-feedstock/issues/70). This resolves #131 .

I still want to do some more testing of this and look into whether there are any tests that are currently disabled with mpiuni that should be enabled. But I have done significant testing with ESMPy and have verified that we can now run ESMPy's test suite with mpiuni and all tests pass in that configuration. So it feels like this is getting close enough that it's worth opening a PR.

@theurich and/or @oehmke - I would welcome a review of this. I am assigning both of you as reviewers but will let you decide if you have the time and interest to review it. I will make some line comments calling out pieces that I think most warrant a careful double-check.

oehmke commented 6 months ago

Sorry for not responding sooner. I'll review this soon (hopefully this week), and we can check if Gerhard also wants to take a look.

billsacks commented 6 months ago

No rush! As you may see from the commit dates, this has been very on-again-off-again work anyway. And I'm actually thinking of putting this on pause so I can work on prototyping the Julia model coupling next, since that feels high priority in terms of time.

billsacks commented 2 weeks ago

I have run testing on my Mac on the latest version (cc470b7362), both with openmpi and mpiuni. All tests pass (using exhaustive testing), and ESMPy tests pass.

billsacks commented 2 weeks ago

I have done enough testing on this that it feels ready to merge, and I'm going to do so in order to get nightly test results.

I still want to add some more unit tests that invoke I/O and are run with mpiuni: I noticed that much of our I/O testing (e.g., ArrayIOUTest and similar) is only set up for multi-processor testing. My thought is to add at least some basic Array I/O tests that are run when testing with mpiuni. I'm thinking that I'll do that by introducing a new test file like ESMF_ArrayIOBasicUTest, where the Array setup will work on single-processor testing. But I want to get this merged to see initial test results, so I'll add that later. @oehmke let me know if you have any thoughts on this plan of adding a new unit test file for the sake of adding some tests that will work with mpiuni: it feels a little weird to me to add a new file for that purpose, but I don't see a way around it other than doing some potentially significant rework of all of the tests in ESMF_ArrayIOUTest to support single-processor testing - since my understanding is that, if any tests in a file only support multi-processor, then ALL tests in the file need to be for multi-processor only.