Features/jet partitions

[christinaholtNOAA]

DESCRIPTION OF CHANGES:

Add the ability for forecast jobs on Jet and Hera to use resources more efficiently, and to allow for Jet jobs to run on more partitions, which will be helpful during the HFIP Allocation Season. This involved additional Slurm tags in the rocoto xml, and changing pre-defined grids' layouts and blocksize to be consistent with

Other changes:

• Converted nthreads to a configurable option in model_configure
• Updated layout and blocksize for GSD_HRRR25km pre-defined grid.
• Changed dependency for post jobs to also allow for completion of run_fcst to trigger jobs.
• Reduce the number of tries for all tasks to 1. Anything higher is wasteful of resources and time, especially when there are forecast tasks known to fail in the test suite.

The concept adopted here is based on tests performed by Sam Trahan on the AVID real-time setup of HRRR. He found that a more efficient way to run the forecast model is by using the following settings:

    <partition>kjet,xjet,sjet,vjet</partition>
    <cores>240</cores>
    <native>--cpus-per-task 4</native>
    <native>--exclusive</native>

along with OMP_NUM_THREADS=4

To adopt these settings in a more general way, he also provided a two rules to follow in order to avoid crashes when choosing layouts and blocksize:

blocksize <= grid_size_y / layout_y / 2
blocksize >= grid_size_x / layout_x / 2 \

TESTS CONDUCTED:

I ran the test suite (except for nco tests) on both Jet and Hera. Applying the above techniques, I fixed a couple of the tests that were already failing for develop -- regional_003, regional_004, and new_JPgrid. (After merging with develop, I realize that new_JPgrid may have already been fixed)

CONTRIBUTORS:

@SamuelTrahanNOAA, Dom Heinzeller, and several colleagues in GSL/ATD.

[JeffBeck-NOAA]

Changes look good to me.

[mkavulich]

These changes look good; @christinaholtNOAA ping me when you have resolved the merge conflicts and I will do a quick re-test and merge (presuming no problems).

Side note: Ran a 6-hour forecast of the GSD_HRRR25km domain on Cheyenne; the model forecast step showed a very significant runtime improvement from 880 seconds to 165 seconds...it's even faster than the make_ics task now!

[christinaholtNOAA]

Very nice result! To be clear, I didn't change any submit directives at all for Cheyenne. The only thing that should be affecting that speedup is the layout of the grid!

I should get to the conflicts and re-tests tomorrow.

[christinaholtNOAA]

Need some feedback on #245 before I can resolve conflicts. @JeffBeck-NOAA @gsketefian

[gsketefian]

@christinaholtNOAA This looks good to me.

An indirectly related question:
I was planning on removing the test in setup.sh on BLOCKSIZE at some point that checks whether BLOCKSIZE is evenly divisible by the number of horizontal grid points (i.e. number of columns) in an MPI task, i.e. given

nx_per_task=$(( NX/LAYOUT_X ))
ny_per_task=$(( NY/LAYOUT_Y ))
num_cols_per_task=$(( $nx_per_task*$ny_per_task ))
rem=$(( num_cols_per_task%BLOCKSIZE ))

it checks whether rem is 0. I think Dom mentioned during yesterday's deliverables meeting that the code is faster if this test is satisfied (but it won't crash if it's not, like it used to). Is that your understanding as well?

[christinaholtNOAA]

@gsketefian I cannot speak to the stability of those types of changes. I think it could help to leave it and provide a bit more guidance on how to choose these numbers for efficiency. Is there some need to remove the check?

[gsketefian]

Well we originally put it in that check because if BLOCKSIZE wasn't set in that specific way, the forecast would stop but the executable would still be going and would waste all your core-hours. Then Dom fixed things so that the code can run without BLOCKSIZE satisfying that condition, so the test was no longer relevant. But I think he mentioned on Wednesday that the code runs a lot faster if the condition on BLOCKSIZE is satisfied.

I won't change anything for now, but somewhere in the docs we should mention the condition you mentioned in this PR [i.e.
that it's best to have grid_size_x / layout_x / 2 <= blocksize <= grid_size_y / layout_y / 2] as well as Dom's condition (I'll have to ask him again to make sure I understood him right). I'll make an issue later today so we don't forget.

[christinaholtNOAA]

@mkavulich I have resolved my conflicts. What testing should be done now?

[mkavulich]

@christinaholtNOAA I'm running a test on Cheyenne now, I'll merge once that test is complete

[mkavulich]

The suite of Hera tests mostly completed (including 3 and 4 which previously failed!), and Cheyenne test was also successful.

[SamuelTrahanNOAA]

This is wrong:

blocksize <= grid_size_y / layout_y / 2
blocksize >= grid_size_x / layout_x / 2 \

The inequality for the x direction is backwards. I don't know where the inequality for y comes from, nor whether it is true.

[christinaholtNOAA]

Yep! Nice catch on the x-direction. Thanks, @SamuelTrahanNOAA. I believe I should have left it as:

blocksize = grid_size_x / layout_x / 2

For the y-direciton, I was referencing our emails (thread: how not to blow up GSL physics):

Another thing that reliably causes crashes is having a blocksize that is too large. Your limit is:

blocksize <= grid_size_y / layout_y / 2

[SamuelTrahanNOAA]

blocksize <= grid_size_x/layout_x/2

You get warnings if grid_size_x/layout_x/2 is not divisible by the blocksize. I've seen FV3 crash in those cases too.

Small blocksizes tend to be extremely slow and very small ones will sometimes crash.