search

payu-org / payu

A workflow management tool for numerical models on the NCI computing systems
Apache License 2.0
18 stars 25 forks source link

Remove create_mask_table function from mom5 driver #352

Open aidanheerdegen opened 11 months ago

aidanheerdegen commented 11 months ago

The create_mask_table function is never used so I don't know if it still works.

https://github.com/payu-org/payu/blob/c8e74243dd27c42129901efbcc82916e8113da84/payu/models/mom.py#L130-L226

This comment suggests it isn't currently useable.

https://github.com/payu-org/payu/blob/c8e74243dd27c42129901efbcc82916e8113da84/payu/models/mom.py#L87-L90

Personally I find the creating mask tables to be a little fragile. Sometimes it just doesn't produce mask tables that work correctly, and so need a little hand editing to remove a masked cell.

From some old notes:

[aph502@raijin6 sis01_mosaic]$ pwd /home/502/aph502/v45_short/sis01_mosaic

../manual_testing/mom/src/tools/check_mask/check_mask --grid_file [ocean_mosaic.nc](http://ocean_mosaic.nc/) --ocean_topog [topog.nc](http://topog.nc/) --layout 72,75

[aph502@raijin6 v45_short]$ cp sis01_mosaic/mask_table.1423.72x75 sis01_inputs_5400/ice_mask_table
[aph502@raijin6 v45_short]$ cp sis01_mosaic/mask_table.1423.72x75 sis01_inputs_5400/ocean_mask_table

Added to - /short/v45/aph502/sis01_inputs_5400 inputs in config.yaml

And copied the diag_table from the 3600 CPU run, i.e. minimal diagnostics.

This threw up an error 

MPP_DEFINE_DOMAINS(mpp_compute_extent): domain extents must be positive definite.

Looking in mpp_compute_extent in https://github.com/BreakawayLabs/mom/blob/949d96dde8cc0648d0d39ab60507827823651b02/src/tools/shared/mpp_domain.c

I wondered if it was the odd number of domains after masking. So I edited the mask files, unmasked the first entry in both, 
added one to the number of CPUS and it ran fine!
marshallward commented 11 months ago

While I think it was a good idea, the CPU count problem was not really solvable. I would not miss it if you decide to remove it.

aidanheerdegen commented 11 months ago

While I think it was a good idea, the CPU count problem was not really solvable

Would it work as a stand-alone utility?

marshallward commented 11 months ago

Hard to say, I haven't looked at it in years! But I think most of it is just fetching the arguments from the input files and passing them to check_mask. Even then, I think the main goal was to determine the number of land domains to be subtracted from the layout.

I don't think there's much here, I think it can go.

aidanheerdegen commented 11 months ago

Should remove this as part of this clean-up

https://github.com/payu-org/payu/blob/master/payu/experiment.py#L133-L134