Add example of regional configuration using regional-mom6 package forced with ACCESS-OM2-01 #338

navidcy · 2024-04-23T07:33:46Z

This closes COSIMA/regional-mom6#59

cc @ashjbarnes

review-notebook-app · 2024-04-23T07:33:51Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

navidcy · 2024-04-23T07:55:23Z

We need to ensure the example runs.

Also we need to modify so that it uses the cosima-cookbook to load ACCESS-OM2 data; see COSIMA/regional-mom6#131

navidcy · 2024-04-25T08:27:46Z

@ashjbarnes I tried to run this and it errors on initial condition. See the output of Step 5 cell at https://app.reviewnb.com/COSIMA/cosima-recipes/pull/338/

navidcy · 2024-05-03T12:10:46Z

@ashjbarnes is this error related to the bug in COSIMA/regional-mom6#170? If so, it means that https://github.com/COSIMA/regional-mom6/releases/tag/v0.5.1 would be required.

cc @luweiyang

navidcy · 2024-05-06T07:55:48Z

I tried to see if regional-mom6 v0.5.2 healed the issues I was having. It didn't... :(

luweiyang · 2024-05-09T23:19:13Z

@ashjbarnes I tried to run the ACCESS-OM2 forced case and realised that the west and east unprocessed boundary files west_unprocessed.nc and east_unprocessed.nc are missing dimensions xu_ocean and xt_ocean (north_unprocessed.nc and south_unprocessed.nc have these two dimensions and can be processed by expt.rectangular_boundary )
xu_ocean = UNLIMITED ; // (0 currently)

luweiyang · 2024-05-10T05:52:44Z

@ashjbarnes @navidcy

I've fixed the west and east boundary condition issues. I changed cell 6 in step 2 (changes in bold):
rmom6.longitude_slicer(om2_input, [longitude_extent[1]-0.2, longitude_extent[1]+0.2], ["xu_ocean", "xt_ocean"]).to_netcdf(tmp_dir + "/east_unprocessed.nc")

The notebook is running fine now and I've pushed the working version to the ncc/regional-mom6-v2 branch.

However, I couldn't run the experiment as I keep getting warning & error messages about initialization and extreme surface state. So somehow the date of ocean forcing and atmospheric forcing do not match? Since we use repeated year forcing, I didn't think that would be a problem... Any thoughts? And it turns out that the temperature unit is K (instead of C) in init_tracers.nc.

 Entering coupler_init at 20240510 154506.457
 Starting initializing ensemble_manager at 20240510 154506.458
 Finished initializing ensemble_manager at 20240510 154506.458
 Starting to initialize diag_manager at 20240510 154506.489

WARNING from PE     0: diag_manager_mod::diag_manager_init: DIAG_MANAGER_NML not found in input nml file.  Using defaults.

 Finished initializing diag_manager at 20240510 154506.493
 Starting to initialize tracer_manager at 20240510 154506.502
 Finished initializing tracer_manager at 20240510 154506.512
 Beginning to initialize component models at 20240510 154506.512
 Starting to initialize atmospheric model at 20240510 154506.534
 Finished initializing atmospheric model at 20240510 154506.581
 Starting to initialize land model at 20240510 154506.595
 Finished initializing land model at 20240510 154506.607
 Starting to initialize ice model at 20240510 154506.636
 Finished initializing ice model at 20240510 154506.731
 Starting to initialize ocean model at 20240510 154506.739

WARNING from PE    54: Extreme surface sfc_state detected: i=  57 j= 128 lon= 145.825 lat= -43.366 x= 145.825 y= -43.366 D= 8.3479E+01 SSH= 5.6639E-02 SST= 2.8748E+02 SSS= 3.4988E+01 U-= 2.0583E-02 U+= 2.0757E-02 V-=-6.1662E-03 V+= 4.9824E-03

WARNING from PE    54: There were more unreported extreme events!

FATAL from PE    39: There were a total of     29869 locations detected with extreme surface values!

navidcy · 2024-05-10T08:12:23Z

Great!

something else we’d want to do is to use the cookbook to load the ACCESS-OM2 output

navidcy · 2024-05-10T10:36:36Z

DocumentedExamples/regional-mom6-forced-by-access-om2.ipynb

@@ -0,0 +1,699 @@
+{


why there are 3 commented out lines of code @luweiyang?
if we need them, then uncomment,
if we don't need them then delete?

Reply via ReviewNB

I don't think we need them. L20 and L21 are the same as L6 and L7 for selecting data along the dims yu_ocean and yt_ocean. I will delete them.

Generally, commented code is bad practice...
Either we want the code or we delete it.
It's good to let go. :)

navidcy · 2024-05-10T10:37:19Z

DocumentedExamples/regional-mom6-forced-by-access-om2.ipynb

@@ -0,0 +1,699 @@
+{


why was this cell commented out @luweiyang? is the dask client create issues?

Reply via ReviewNB

No, I commented them out when I was debugging because at some stage I did get some client-related errors. But it's no longer an issue. I will uncomment them.

navidcy · 2024-05-10T10:39:07Z

something else we’d want to do is to use the cookbook to load the ACCESS-OM2 output

this is related to loading from cookbook: COSIMA/regional-mom6#131

luweiyang · 2024-05-10T11:42:35Z

Yes, I checked the date as well and was confused.

As @angus-g pointed out, the data in this folder
/g/data/ik11/outputs/access-om2-01/01deg_jra55v13_ryf9091/output1077/
is for year 2170, it goes from 2170/ 4/ 1 0: 0: 0 to 2170/ 7/ 1 0: 0: 0 (you can see this info in access-om2.out). Looks like we're actually getting boundary conditions from April in year 2170 as our ocean forcing?

The info for the atmospheric forcing is provided in data_table:
"ATM", "u_bot", "uas_10m", "./INPUT/RYF.u_10.1990_1991.nc", "bicubic", 1.0
Maybe we should update the year in filename to 2003_2004 here? Or it wouldn't matter since it's repeated year forcing?

Does this mismatch in year explain the initialisation error?

ashjbarnes · 2024-05-10T13:04:27Z

I've fixed the west and east boundary condition issues. I changed cell 6 in step 2 (changes in bold): rmom6.longitude_slicer(om2_input, [longitude_extent[1]-0.2, longitude_extent[1]+0.2], ["xu_ocean", "xt_ocean"]).to_netcdf(tmp_dir + "/east_unprocessed.nc")

The notebook is running fine now and I've pushed the working version to the ncc/regional-mom6-v2 branch.

However, I couldn't run the experiment as I keep getting warning & error messages about initialization and extreme surface state. So somehow the date of ocean forcing and atmospheric forcing do not match? Since we use repeated year forcing, I didn't think that would be a problem... Any thoughts? And it turns out that the temperature unit is K (instead of C) in init_tracers.nc.

Yes good find! It'll be crashing because of the temperatures being in Kelvin. This is strange - these lines of code check the minimum surface temperature value of the dataset and then assume that it's in Kelvin if this minimum is above 100. This used to work but apparently not any more! The code here hasn't been changed for a long time

This notebook worked fine a while ago but it was neglected whilst we made a lot of changes. Somewhere we've introduced a change that breaks it but I don't know where..

navidcy · 2024-05-10T13:11:10Z

This is good. It will force us to find whe K->C doesn’t work and implement a test to capture future similar failures.

Let’s not ruminate on the past. Sure it worked at some point. It doesn’t matter that much, unless it helps us find why it’s not working now. Let’s just make an example that works now. It doesn’t even have to resemble how it was in the past if we want to showcase something else.

navidcy · 2024-05-10T13:13:24Z

Yes, I checked the date as well and was confused.

As @angus-g pointed out, the data in this folder /g/data/ik11/outputs/access-om2-01/01deg_jra55v13_ryf9091/output1077/ is for year 2170, it goes from 2170/ 4/ 1 0: 0: 0 to 2170/ 7/ 1 0: 0: 0 (you can see this info in access-om2.out). Looks like we're actually getting boundary conditions from April in year 2170 as our ocean forcing?

The info for the atmospheric forcing is provided in data_table: "ATM", "u_bot", "uas_10m", "./INPUT/RYF.u_10.1990_1991.nc", "bicubic", 1.0 Maybe we should update the year in filename to 2003_2004 here? Or it wouldn't matter since it's repeated year forcing?

Does this mismatch in year explain the initialisation error?

If we are gonna load the RYF then there is no point referring to “year 2003” or anything. All years are the same! They are looped.

I do think it more intuitive if we force with IAF, some particular year that we load via the cookbook

ashjbarnes · 2024-05-10T13:21:23Z

This is good. It will force us to find whe K->C doesn’t work and implement a test to capture future similar failures.

Let’s not ruminate on the past. Sure it worked at some point. It doesn’t matter that much, unless it helps us find why it’s not working now. Let’s just make an example that works now. It doesn’t even have to resemble how it was in the past if we want to showcase something else.

Knowing that it worked in the past should help us troubleshoot though right? Nothing about the forcing dataset has changed, only our code. If our code used to correctly convert K to C and no longer does, that means something in the package (specifically something in the initial condition method) has changed that prevents it from doing this.

I've had a quick look in case there was a premature "fill nans with 0s" or something but that doesn't seem to be the case

luweiyang · 2024-05-10T21:48:54Z

Yes, same ocean forcing year and same atmos forcing did work in the past. I remember a while ago @ashjbarnes updated and recompiled MOM6, and with new exe file everything worked fine.

navidcy · 2024-05-11T05:03:10Z

I never doubted that it worked in the past.. But now it's not, so let's fix it.

There are seem to be at least 1 or more issues. One is that temperature is not converted to K->C despite there is a method that supposedly does that. Possibly:

the method is never called, or
the method does not do what we think it does

How does ERA5-forced example work? Is ERA5 in degrees C to begin with?

navidcy · 2024-05-11T05:26:20Z

I've had a quick look in case there was a premature "fill nans with 0s" or something but that doesn't seem to be the case

This statement is unclear to me. I don't get what you want to say here @ashjbarnes.
"Premature" implying that it shouldn't be there... and now it "doesn't seem to be the case" so that mean "now it's not there anyway" or does it mean that "the fill nans with 0s is not premature" anymore?

…east west b.c.

luweiyang · 2024-05-13T02:34:16Z

I've updated the notebook (uncomment client cell and delete the commented lines). I noticed that temperature in the unit of K in unprocessed initial and boundary conditions (*_unprocessed.nc) in the tmp_dir, and also in forcing_obc_segment_00?.nc (so both initial and forcing conditions). So none of these lines (-=273.15) worked for some reason? @ashjbarnes

It would be nice to make this version work (for output1077 and RYF) and then try to read a particular year of forcing using cookbook.

navidcy · 2024-05-13T03:57:51Z

Thanks @luweiyang.

OK, agreed, let's fix everything else and we convert to reading output via cookbook later.

What's the plan for finding out where the K->C conversion fail?

luweiyang · 2024-05-13T06:35:14Z

@navidcy @ashjbarnes
As far as I can tell, in this notebook, the unprocessed forcing files are obtained via longitude_slicer;
The temperature conversion is done in, for example, initial_condition (in regional_mom6.py);
But initial_condition is not called in longitude_slicer, which may be why the temperature data was not checked and corrected?

navidcy · 2024-05-13T06:52:59Z

@navidcy @ashjbarnes As far as I can tell, in this notebook, the unprocessed forcing files are obtained via longitude_slicer; The temperature conversion is done in, for example, initial_condition (in regional_mom6.py); But initial_condition is not called in longitude_slicer, which may be why the temperature data was not checked and corrected?

longitude_slicer just slices the fields in longitude. It would be very counterintuitive if it was expected to call initial_condition.

@luweiyang are you suggesting that the notebook never ends up calling the jnitial_condition method?

luweiyang · 2024-05-13T07:03:03Z

No sorry, initial_condition was called in step 5. So that's not the reason.

ashjbarnes · 2024-05-13T07:16:14Z

I've had a quick look in case there was a premature "fill nans with 0s" or something but that doesn't seem to be the case

This statement is unclear to me. I don't get what you want to say here @ashjbarnes. "Premature" implying that it shouldn't be there... and now it "doesn't seem to be the case" so that mean "now it's not there anyway" or does it mean that "the fill nans with 0s is not premature" anymore?

Sorry to clarify:

The line that tries to convert K to C checks whether the minimum is greater than 100. This means that if NaNs have been filled in with zeros before this step then the check would pass regardless of the data being C or K. I read through the code and didn't see that this was the case.

To debug we could print what the minimum of the temperature field actually returns, or copy the code into a notebook and return the temperature field at this step to see what it looks like.

Or, instead of fixing it, a quick fix could be to add a "units" flag for temperature to both IC and BC functions so the user just has to tell us when it needs to do the conversion rather than trying to do so automatically.

navidcy · 2024-05-13T07:45:39Z

Hi both, it's helpful to discuss and read the code but we are not compilers and often just by "reading the code" we (or at least I) fail to capture what is actually going on. So to be certain of what's happening here we need to properly debug. To do that we should modify the regional-mom6 source code and install the local copy of the regional-mom6 in the environment so that the notebook uses that. Only this way we (or at least I) would be able to pin-point the source of the matter and fix it. Thoughts?

Or, instead of fixing it, a quick fix could be to add a "units" flag for temperature to both IC and BC functions so the user just has to tell us when it needs to do the conversion rather than trying to do so automatically.

I suggest we "properly fix it" rather than doing a "quick fix". There must be some error in the logic and it's good to make this robust. Note that in addition, my personal experience suggests that a proper fix ends up saving everyone (developers and usrers) much more time compared to what a quick fix does.

luweiyang · 2024-05-14T07:26:17Z

I did a quick test and the if condition for checking the min temperature was not satisfied.

I used an old environment (analysis3-23.07) where the regional_mom6 package wasn't installed. I imported the package from my home directory. This way I can modify the code and see what it does.

I added a line print('xxxx') following the if statement to check this part of the code runs. I can confirm that the if condition was not satisfied for some reason so the temperature unit was not changed.

navidcy · 2024-05-14T07:50:14Z

Can you post here what you did? A minimal example of a notebook loading the temperature and trying to reproduce what the package does demonstrating that it fails?

navidcy · 2024-05-14T07:51:55Z

I used an old environment (analysis3-23.07) where the regional_mom6 package wasn't installed. I imported the package from my home directory. This way I can modify the code and see what it does.

You can always install a local copy of the package even if it’s already installed in the environment. You just need to ensure that you are using your local copy and not the version in the env. I’ll try to demonstrate that and post the code here

navidcy · 2024-05-14T09:47:55Z

Could this be the culprit?

>>> import numpy as np
>>> temp_K = np.array([270., 272., 287.])
>>> temp_K
array([270., 272., 287.])
>>> np.min(temp_K)
270.0
>>> np.min(temp_K) > 100
True

But if this was masked, then]

>>> import numpy as np
>>> temp_K_masked = np.array([270., float('nan'), 287.])
>>> temp_K_masked
array([270.,  nan, 287.])
>>> np.min(temp_K_masked)
nan
>>> np.min(temp_K_masked) > 100
False

?

If it is, then the solution would be to use numpy.nanmin:

>>> np.nanmin(temp_K_masked)
270.0
>>> np.nanmin(temp_K_masked) > 100
True

ashjbarnes · 2024-05-14T09:53:35Z

Nice that could be it!!

navidcy · 2024-05-14T09:57:57Z

@luweiyang could you check that with the change suggested in COSIMA/regional-mom6#176 the issue goes away?

move example from regional-mom6 repo

4425931

navidcy added the 🐣 new example label Apr 23, 2024

This was referenced Apr 23, 2024

Mostly ERA5-GLORYS demo notebook tweaks COSIMA/regional-mom6#160

Merged

Load input using cosima_cookbook COSIMA/regional-mom6#131

Closed

navidcy mentioned this pull request Apr 23, 2024

Adds interactive domain selection via a map in the ACCESS-OM2 demo COSIMA/regional-mom6#140

Open

attempt to run, error on IC

abc1f31

navidcy mentioned this pull request May 3, 2024

Convert demo that uses ACCESS-OM2 output to cosima-recipes example COSIMA/regional-mom6#59

Closed

navidcy assigned luweiyang May 5, 2024

update syntax for regional-mom6 v0.5.2 -- still issues w west boundary

c82d28b

west and east boundary files missing dimensions xu_ocean and xt_ocean

44ade40

West and east boundary issues are fixed

484b349

navidcy commented May 10, 2024

View reviewed changes

Small fixes: restore client and delete unnecessary lines for getting …

6a629c5

…east west b.c.

navidcy mentioned this pull request May 14, 2024

Fixes K->C conversion in initial_condition COSIMA/regional-mom6#176

Open

trying to debug initial_condition

cf4c4e5

Add example of regional configuration using regional-mom6 package forced with ACCESS-OM2-01 #338

Are you sure you want to change the base?

Add example of regional configuration using regional-mom6 package forced with ACCESS-OM2-01 #338

Conversation

navidcy commented Apr 23, 2024 • edited

review-notebook-app bot commented Apr 23, 2024

navidcy commented Apr 23, 2024

navidcy commented Apr 25, 2024

navidcy commented May 3, 2024 • edited

navidcy commented May 6, 2024

luweiyang commented May 9, 2024

luweiyang commented May 10, 2024 • edited

navidcy commented May 10, 2024

navidcy May 10, 2024 • edited

Choose a reason for hiding this comment

luweiyang May 10, 2024

Choose a reason for hiding this comment

navidcy May 10, 2024 • edited

Choose a reason for hiding this comment

navidcy May 10, 2024 • edited

Choose a reason for hiding this comment

luweiyang May 10, 2024

Choose a reason for hiding this comment

navidcy commented May 10, 2024

luweiyang commented May 10, 2024

ashjbarnes commented May 10, 2024 • edited

navidcy commented May 10, 2024

navidcy commented May 10, 2024

ashjbarnes commented May 10, 2024

luweiyang commented May 10, 2024

navidcy commented May 11, 2024

navidcy commented May 11, 2024

luweiyang commented May 13, 2024

navidcy commented May 13, 2024

luweiyang commented May 13, 2024

navidcy commented May 13, 2024 • edited

luweiyang commented May 13, 2024

ashjbarnes commented May 13, 2024

navidcy commented May 13, 2024 • edited

luweiyang commented May 14, 2024

navidcy commented May 14, 2024

navidcy commented May 14, 2024 • edited

navidcy commented May 14, 2024 • edited

ashjbarnes commented May 14, 2024

navidcy commented May 14, 2024

navidcy commented Apr 23, 2024 •

edited

navidcy commented May 3, 2024 •

edited

luweiyang commented May 10, 2024 •

edited

navidcy May 10, 2024 •

edited

navidcy May 10, 2024 •

edited

navidcy May 10, 2024 •

edited

ashjbarnes commented May 10, 2024 •

edited

navidcy commented May 13, 2024 •

edited

navidcy commented May 13, 2024 •

edited

navidcy commented May 14, 2024 •

edited

navidcy commented May 14, 2024 •

edited