add checks of io layout to cmeps driver #496

anton-seaice · 2024-08-29T05:30:36Z

This PR implements checks of the processors selected in the nuopc.runconfig file for the access-om3 payu driver. This ensures that the processors requested for normal model operations and parallel IO are within range of the CPUs set in config.yaml and each model "realm" uses processors for IO that are within the range of processors for that realm.

It adds tests for the min/max bounds of each parameter.

Contributes to COSIMA/access-om3#109

pep8speaks · 2024-08-29T05:30:42Z

Hello @anton-seaice! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file payu/models/cesm_cmeps.py:

Line 193:80: E501 line too long (86 > 79 characters)
Line 194:80: E501 line too long (90 > 79 characters)
Line 195:80: E501 line too long (86 > 79 characters)
Line 196:80: E501 line too long (90 > 79 characters)
Line 199:80: E501 line too long (83 > 79 characters)
Line 205:80: E501 line too long (90 > 79 characters)
Line 211:80: E501 line too long (83 > 79 characters)
Line 227:80: E501 line too long (89 > 79 characters)
Line 231:80: E501 line too long (81 > 79 characters)
Line 234:80: E501 line too long (87 > 79 characters)
Line 235:80: E501 line too long (86 > 79 characters)
Line 242:80: E501 line too long (92 > 79 characters)
Line 243:80: E501 line too long (88 > 79 characters)
Line 245:80: E501 line too long (97 > 79 characters)
Line 248:80: E501 line too long (93 > 79 characters)
Line 251:50: W291 trailing whitespace
Line 256:80: E501 line too long (90 > 79 characters)
Line 261:80: E501 line too long (93 > 79 characters)
Line 266:80: E501 line too long (84 > 79 characters)
Line 267:80: E501 line too long (89 > 79 characters)

In the file test/models/access-om3/test_access_om3.py:

Line 9:80: E501 line too long (87 > 79 characters)
Line 101:80: E501 line too long (86 > 79 characters)
Line 115:80: E501 line too long (86 > 79 characters)
Line 141:80: E501 line too long (86 > 79 characters)
Line 153:80: E501 line too long (94 > 79 characters)
Line 180:80: E501 line too long (86 > 79 characters)
Line 189:80: E501 line too long (84 > 79 characters)
Line 218:80: E501 line too long (86 > 79 characters)
Line 226:80: E501 line too long (88 > 79 characters)
Line 280:80: E501 line too long (80 > 79 characters)

Comment last updated at 2024-09-18 06:20:40 UTC

anton-seaice · 2024-08-29T05:32:02Z

Apologies to the wrong minghangli !! @minghangli-uni, can you look at this please?

coveralls · 2024-08-29T05:33:54Z

coverage: 56.769% (+1.0%) from 55.76%
when pulling a90763b on om3_iolayout
into 2826621 on master.

payu/models/cesm_cmeps.py

minghangli-uni · 2024-08-29T06:27:38Z

Thanks @anton-seaice. I just did payu setup with a "wrong" pio setting, but it doesnt complain.

minghangli-uni · 2024-08-29T22:04:24Z

I've done a few tests on pio_root, pio_numiotasks and pio_stride. One ICE_modelio example below is based on a most recent merge of the 0.25 deg ryf configuration. The ncpus for cice is 96.

ICE_modelio::
     diro = ./log
     logfile = ice.log
     pio_async_interface = .false.
     pio_netcdf_format = nothing
     pio_numiotasks = 3
     pio_rearranger = 1
     pio_root = 100000
     pio_stride = 48
     pio_typename = netcdf4p
::

What I've found is,

pio_root does not appear to impact the I/O tasks as expected, which seems to be a bug. For example, setting pio_root to a very large value, such as 100000, does not affect the run.
Both pio_numiotasks and pio_stride function as intended, and their effects on the I/O tasks are properly reflected. The above example will throw me an error because the third IO task exceeds the range of available CICE cpus (i.e. 96)

dougiesquire

Thanks for adding this @anton-seaice. Now that we're starting to accumulate a few checks in the driver, I think it would be worth putting these in a new _qa_check (or something) method that's called from setup for clarity.

minghangli-uni

Thanks @anton-seaice for your work! I've left some comments. Can you please take a look at those?

payu/models/cesm_cmeps.py

anton-seaice · 2024-08-29T22:52:24Z

I've done a few tests on pio_root, pio_numiotasks and pio_stride. One ICE_modelio example below is based on a most recent merge of the 0.25 deg ryf configuration. The ncpus for cice is 96.
ICE_modelio::
     diro = ./log
     logfile = ice.log
     pio_async_interface = .false.
     pio_netcdf_format = nothing
     pio_numiotasks = 3
     pio_rearranger = 1
     pio_root = 100000
     pio_stride = 48
     pio_typename = netcdf4p
::
What I've found is,
1. `pio_root` does not appear to impact the I/O tasks as expected, which seems to be a bug. For example, setting `pio_root` to a very large value, such as 100000, does not affect the run.

pio_root in this example would be (almost silently) changed to 0 in the pio shared code. It's not a behaviour I love, I think it should just cause an error. Payu would reject this after this PR.

payu/models/cesm_cmeps.py

aidanheerdegen

I have some questions and suggestions.

payu/models/cesm_cmeps.py

Co-authored-by: Aidan Heerdegen <[email protected]>

aidanheerdegen · 2024-09-18T05:53:45Z

Before I forget when this does get merged please either squash when merging, or rebase beforehand to clean up the commit history.

aidanheerdegen

LGTM. I have pinged @minghangli-uni because there are some unresolved conversations that he should check have been resolved and mark them as such. Then good to merge I think.

minghangli-uni

LGTM! Thanks @anton-seaice

payu-org/payu#496 Removed some checks in Payu which required unused components to be present in the nuopc.runconfig file.

payu-org/payu#496 Removed some checks in Payu which required unused components to be present in the nuopc.runconfig file. Co-authored-by: Dougie Squire <[email protected]>

Change ice_ntasks to be 96 Remove unused components payu-org/payu#496 Removed some checks in Payu which required unused components to be present in the nuopc.runconfig file. Co-authored-by: Dougie Squire <[email protected]>

payu-org/payu#496 Removed some checks in Payu which required unused components to be present in the nuopc.runconfig file. Co-authored-by: Dougie Squire <[email protected]>

…ion (#169) * License file * Update incorrect note in MOM_override See COSIMA/access-om3#251 Co-authored-by: minghangli-uni <[email protected]> * Add non-default from MOM_parameter_doc.layout in the middle reorder MOM_input to follow MOM_parameter_doc.short Put non-default values from MOM_parameter_doc.debugging at the top Co-authored-by: Dougie Squire <[email protected]> * Add WIND_STAGGER to MOM_INPUT for clarity This only affects the logging, the actual value is hardcoded through a #ifdef CESMCOUPLED * Change atm and ice mesh from 1deg -> 025deg Change ice_ntasks to be 96 Remove unused components payu-org/payu#496 Removed some checks in Payu which required unused components to be present in the nuopc.runconfig file. Co-authored-by: Dougie Squire <[email protected]> * Update to ACCESS-NRI/ACCESS-OM3 deployment 25.01.0 * Update field dictionary with latest from CMEPS * add_gusts parameter needed for 0.4.0 build * Update nuopc.runconfig to turn-off dates in restart_pointers * Turn on netcdf4 per COSIMA/access-om3#81 The new build uses openmpi4.1.7, which fixes some bugs preventing parallel reads over symlinks. This allows use of parallelio netcdf4 option, even though at this resolution for most components we will still only use one PE for IO. * Use .d and .m in CICE history output filenames Per COSIMA/access-om3#201 * Turn off extra cice restarts and remove non functioning write_restart_at_endofrun per COSIMA/access-om3#175 * Set Earth radius the same as the UM * Update MOM6 parameters and docs See COSIMA/access-om3#274 --------- Co-authored-by: anton-seaice <[email protected]> Co-authored-by: Dougie Squire <[email protected]> Co-authored-by: Anton Steketee <[email protected]> Co-authored-by: dougiesquire <[email protected]>

Change ice_ntasks to be 96 Remove unused components payu-org/payu#496 Removed some checks in Payu which required unused components to be present in the nuopc.runconfig file. Co-authored-by: Dougie Squire <[email protected]>

payu-org/payu#496 Removed some checks in Payu which required unused components to be present in the nuopc.runconfig file. Co-authored-by: Dougie Squire <[email protected]>

anton-seaice self-assigned this Aug 29, 2024

This was referenced Aug 29, 2024

[0.25deg] update PE layout COSIMA/access-om3#214

Closed

[0.25deg] Update PE layout ACCESS-NRI/access-om3-configs#114

Merged

anton-seaice commented Aug 29, 2024

View reviewed changes

payu/models/cesm_cmeps.py Outdated Show resolved Hide resolved

anton-seaice commented Aug 29, 2024

View reviewed changes

payu/models/cesm_cmeps.py Outdated Show resolved Hide resolved

minghangli-uni mentioned this pull request Aug 29, 2024

pio configuration checks COSIMA/access-om3#158

Closed

dougiesquire reviewed Aug 29, 2024

View reviewed changes

minghangli-uni suggested changes Aug 29, 2024

View reviewed changes

payu/models/cesm_cmeps.py Outdated Show resolved Hide resolved

payu/models/cesm_cmeps.py Outdated Show resolved Hide resolved

payu/models/cesm_cmeps.py Outdated Show resolved Hide resolved

payu/models/cesm_cmeps.py Outdated Show resolved Hide resolved

anton-seaice added 9 commits August 30, 2024 13:45

add checks of io layout to cmeps driver

889d632

move to python errors and PEP things

76f2a33

getting the greater checks the right way around

1b351d3

third time lucky

44d84b2

PEP

c937dd6

review feedback and improvements for corner cases

f572ba8

tidy

74ca45a

whitespace

673b043

runconfig_get_component_list test

5facb60

anton-seaice force-pushed the om3_iolayout branch from 7b9bff6 to 5facb60 Compare August 30, 2024 03:45

anton-seaice added 6 commits August 30, 2024 15:59

First pass of tests

3871403

tidy and PEP

9a6c663

PEP

511a462

PEP

75311eb

PEP

7176be8

PEP

673bbee

anton-seaice commented Sep 11, 2024

View reviewed changes

payu/models/cesm_cmeps.py Outdated Show resolved Hide resolved

Update payu/models/cesm_cmeps.py

94760c3

aidanheerdegen reviewed Sep 16, 2024

View reviewed changes

anton-seaice and others added 2 commits September 16, 2024 20:13

Update payu/models/cesm_cmeps.py

b954482

Co-authored-by: Aidan Heerdegen <[email protected]>

Review comments

8dddc49

anton-seaice added 4 commits September 18, 2024 16:00

review comments & PEP

25c8c31

review & PEP

9ec70a6

PEP

09bee92

PEP

a90763b

aidanheerdegen requested review from aidanheerdegen and minghangli-uni September 23, 2024 23:49

aidanheerdegen approved these changes Sep 23, 2024

View reviewed changes

minghangli-uni approved these changes Sep 24, 2024

View reviewed changes

anton-seaice merged commit eb8e8f8 into master Sep 24, 2024
8 checks passed

anton-seaice deleted the om3_iolayout branch September 24, 2024 00:57

anton-seaice added a commit to ACCESS-NRI/access-om3-configs that referenced this pull request Jan 31, 2025

Remove unused components

f9bd1cf

payu-org/payu#496 Removed some checks in Payu which required unused components to be present in the nuopc.runconfig file.

anton-seaice mentioned this pull request Jan 31, 2025

Use payu 1.1.6 in CI ACCESS-NRI/access-om3-configs#166

Merged

anton-seaice added a commit to ACCESS-NRI/access-om3-configs that referenced this pull request Jan 31, 2025

Remove unused components

b99d1b8

payu-org/payu#496 Removed some checks in Payu which required unused components to be present in the nuopc.runconfig file.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add checks of io layout to cmeps driver #496

add checks of io layout to cmeps driver #496

anton-seaice commented Aug 29, 2024 •

edited

Loading

pep8speaks commented Aug 29, 2024 •

edited

Loading

anton-seaice commented Aug 29, 2024

coveralls commented Aug 29, 2024 •

edited

Loading

minghangli-uni commented Aug 29, 2024

minghangli-uni commented Aug 29, 2024 •

edited

Loading

dougiesquire left a comment •

edited

Loading

minghangli-uni left a comment

anton-seaice commented Aug 29, 2024

aidanheerdegen left a comment

aidanheerdegen commented Sep 18, 2024

aidanheerdegen left a comment

minghangli-uni left a comment

add checks of io layout to cmeps driver #496

add checks of io layout to cmeps driver #496

Conversation

anton-seaice commented Aug 29, 2024 • edited Loading

pep8speaks commented Aug 29, 2024 • edited Loading

Comment last updated at 2024-09-18 06:20:40 UTC

anton-seaice commented Aug 29, 2024

coveralls commented Aug 29, 2024 • edited Loading

minghangli-uni commented Aug 29, 2024

minghangli-uni commented Aug 29, 2024 • edited Loading

dougiesquire left a comment • edited Loading

Choose a reason for hiding this comment

minghangli-uni left a comment

Choose a reason for hiding this comment

anton-seaice commented Aug 29, 2024

aidanheerdegen left a comment

Choose a reason for hiding this comment

aidanheerdegen commented Sep 18, 2024

aidanheerdegen left a comment

Choose a reason for hiding this comment

minghangli-uni left a comment

Choose a reason for hiding this comment

anton-seaice commented Aug 29, 2024 •

edited

Loading

pep8speaks commented Aug 29, 2024 •

edited

Loading

coveralls commented Aug 29, 2024 •

edited

Loading

minghangli-uni commented Aug 29, 2024 •

edited

Loading

dougiesquire left a comment •

edited

Loading