Add parallel I/O output system#713
Conversation
4c009fd to
b226a95
Compare
beefeab to
0d53f58
Compare
Co-authored-by: Lachlan Whyborn <lachlan.s.whyborn@gmail.com>
This is done to allow access to private components of derived types in child submodules. See this bug report for more details: https://community.intel.com/t5/Intel-Fortran-Compiler/Intel-oneAPI-bug-with-submodules/td-p/1347530
0d53f58 to
cfb572c
Compare
cfb572c to
458d67d
Compare
458d67d to
df516f5
Compare
|
Hi @abhaasgoyal @Whyborn, thank you for your time in reviewing these PRs! This one is now ready for review, and no rush of course, it's the heaviest one so far! As an introduction, it might be useful to read these developer documentation pages:
Benchcab is running currently - I had to make a small change to achieve bitwise compatibility, I will update the status soon. Thank you again! |
Whyborn
left a comment
There was a problem hiding this comment.
Done a first pass. My main issue at the moment is with the structure of the output_variable_t definition. The current structure is amenable with what already existed within CABLE, but I don't see it being amenable to the planned user output API. The current structure will lead to a combinatory explosion of the number of output variables defined, e.g. a variable with 2 allowed reductions would require 15 output_variable_t, for each combination of reduction (3 including no reduction) and aggregation method (5). These would be allocated regardless of whether the variable is written or not.
I think it would make more sense to have a variable definition, which has only one instance per internal variable. It would contain a reference to the target variable, dimensionality and attributes for the variable. Then we use this and the information supplied by the user's output definition to only define the output variables that are actually going to be written.
… new dimension in top level output module docs
|
Hi @abhaasgoyal and @Whyborn, thanks again for taking a look at this! As per our meetings, I removed the As I mentioned, there are some changes that should follow after this PR. These were left out either because they broke bitwise compatibility or because they were too outside the scope of the PR. These are:
Let me know if you any further questions or comments on this! Many thanks |
|
Which configurations have you used to check bitwise repro? And have you tested with the various current output options available in the Since this PR commit removes the old system, just want to make sure we don't break existing setups before introducing the new API. |
@Whyborn thanks for reminding me about this! I'm now checking the grid and frequency settings for both flux tower and spatial using benchcab: science_configurations:
- cable:
output:
grid: land
averaging: all
- cable:
output:
grid: land
averaging: user6
- cable:
output:
grid: land
averaging: daily
- cable:
output:
grid: land
averaging: monthly
- cable:
output:
grid: mask
averaging: all
- cable:
output:
grid: mask
averaging: user6
- cable:
output:
grid: mask
averaging: daily
- cable:
output:
grid: mask
averaging: monthlyI'll try out a similar method for testing combinations of output variable groups. More to come. |
This change brings in a new output system based on the parallel I/O infrastructure introduced in #706, and is a direct replacement of the previous output module used for offline CABLE. The main motivation behind this is to add MPI support to the serial offline driver, and eventually, to replace the legacy MPI implementation (#358). This also makes progress towards the proposed output redesign (#715) by introducing the underlying data structures needed for its implementation.
The new output system brings in the following enhancements:
This change should be brought in after #712.
Type of change
Please delete options that are not relevant.
Checklist
Testing
CABLE benchcab runs tested using
ifort2021.10.0.Please add a reviewer when ready for review.
📚 Documentation preview 📚: https://cable--713.org.readthedocs.build/en/713/