Fb coupling fields #285

ukmo-juan-castillo · 2021-01-06T11:03:01Z

Changes to tackle issue #206 - addition of new coupling fields

In the original code a coupling lag had to be used, coupling took place at the last time step but not at the first, and the coupling fields had to be written in an oasis restart file. With this ticket, the program can run in a different way by not using a coupling lag, coupling taking place at the first time step but not the last, and the coupling fields are written in the wave restart file - no extra oasis restart file needed.

Enhancement to improve the way in which input data is read in and logged by the ww3_uprstr program: * read in variables specific to the update process selected * output the values provided in the ww3_uprstr.out log file * update the .inp template file and regtests to improve clarity and work with the changes * add capability to read inputs from a namelist (ww3_uprstr.nml) file

ukmo-juan-castillo · 2021-01-06T11:13:00Z

This branch add new send and receive coupling fields to WW3. In the case of receive fields (atmospheric momentum and air density), this has been done in the same way as the already existing received fields (e.g. 10m winds), so there is also the possibility to read these new fields from a file. This implies that the fixed format ww3_sheld.inp and ww3_multi.inp files needs to change to accommodate such changes. ww3_outf.inp files also need to change due to the new diagnostics.

Scientific modifications for air density are already taken into account, so that when no air density is received the current, constant air density value is used in calculations. On the other hand, scientific changes to drive the model with atmospheric momentum are currently under research, so these changes will be added in a later branch once they are sufficiently mature. What is implemented here for atmospheric momentum is only the basic infrastructure to ingest this information in the model.

ukmo-juan-castillo · 2021-01-06T11:28:10Z

The way the ww3_shel file was read was inconsistent if the file was fixed-format or namelist based - one allowed coupling ice concentrations and the other did not. Now it is possible to couple ice concentrations in all cases, and the way how the flags for received fields is processed has been slightly simplified. Furthermore, the piece of code that called the routine to generate an homogeneous ice concentration field was wrong and it has been fixed.

ukmo-juan-castillo · 2021-01-06T11:38:26Z

New mandatory switches (ATTN and ATXN) where added mirroring the behaviour of WNTN and WNXN for the new atmospheric momentum input field. It has been later considered that, to reduce the number of changes required for a run after this branch is merged into the trunk, the wind switches will be reused for momentum. If necessary, this can be changed in the future branch that will add the scientific changes required for driving the model with atmospheric momentum instead of winds. Possibly, checks to ensure than only one of momentum or wind is used to drive the model should also be added.

ukmo-juan-castillo · 2021-01-06T15:59:26Z

Further changes:

. In the subroutine w3mpio, the variable NRQMAX has been modified to properly account for all output fields, and written as an easier to understand sum of terms.

. Small fix to the reading of currents and ice fraction in the restart file in routine w3iors, as these fields have dimension 1:NSEA in each processor.

. The air density is time interpolated when read from file, while other scalar fields (such as ice fraction of water levels) are not. It was implicitly assumed in the code that interpolating fields had to be vector fields, but this has been changed so that the second vector component is only processed if needed.

regarding last modification date

model/ftn/w3iogomd.ftn

model/ftn/w3iorsmd.ftn

model/ftn/w3fld1md.ftn

ukmo-ccbunney

Nice work, Juan.
Just a few comments and changes.
I've not checked all the changes to the input files in the regtests, just the first few; I assume they are all similar.

model/ftn/w3fldsmd.ftn

ukmo-ccbunney · 2021-01-12T09:37:59Z

model/ftn/w3ounfmetamd.ftn

+      META(1)%ENAME  = '.taua'
+      META(1)%VARNM='utaua'
+      META(1)%VARNL='eastward_atm_momentum'
+      META(1)%VARNS='eastward_atm_momentum'


Are the "standard_names" you have added for your new fields actual CF standard names? If not, then the policy we are adopting at the moment is to leave the standard name blank until one has been added to the CF convention. @ukmo-ansaulter is liaising with CF group to achieve this.

I will check if there is a CF standard name and update accordingly.

The relevant standard names will be, I think:

surface_downward_eastward_stress, surface_downward_northward_stress

These have been submitted by another group, are currently under review in the CF discussion forum and haven't been published yet. So I'd suggest leaving blank for the time being and we can apply once the names are confirmed and the CF tables are updated

Makes sense to me, thanks. There is indeed an air_density field, but not a 'mean wave number'.

@ukmo-juan-castillo - I see you have removed the standard_name for "mean_wave_number" as per my comment. However, I think you can leave in the "long_name" value though as that does not require a CF compliant value and will provide a useful description of the field. @ukmo-ansaulter - do you agree?

The same applies to your new TOC field (IFI=6, IFJ=13), lines 2623 and 2631.

I've added specific review comments to the lines.

That's correct @ukmo-ccbunney . In CF the long_name is "free form" so we should always include something. The standard_name has the strict criteria and we should not have one unless there is a match in the latest CF standard name table.

This should avoid a fail when working with any CF compliance checkers...

ukmo-ccbunney · 2021-01-12T09:41:50Z

model/ftn/w3ounfmetamd.ftn

+      META(1)%VARNS='eastward_atm_momentum'
+      META(1)%VARNG='eastward_atm_momentum'
+      META(2)%VARNC='taua=sqrt(utaua**2+vtaua**2)'
+      META(1)%VMIN = -32000


Valid min/max in this module should not be scaled by the scale_factor, but specified in the actual units of the field. I.E. for this field, vmin=-320, not -32000

I will update all these VMIN and VMAX lines.

model/ftn/w3ounfmetamd.ftn

ukmo-ccbunney · 2021-01-12T09:48:05Z

model/ftn/w3ounfmetamd.ftn

+      META(1)%VARNS='mean wave number'
+      META(1)%VARNG='mean wave number'
+      META(1)%VMIN = 0
+      META(1)%VMAX = 32000


same issue here with vmax, should be 32.0, not 32000

model/ftn/w3ounfmetamd.ftn

ukmo-ccbunney · 2021-01-12T11:38:47Z

model/ftn/ww3_shel.ftn

@@ -2206,7 +2226,7 @@
              IF ( FLH(J) ) THEN
                CALL W3FLDH (J, NDST, NDSEN, NX, NY, NX, NY,    &
                             TIME0, TIMEN, NH(J), NHMAX, THO, HA, HD, HS,&
-                             TW0, WX0, WY0, DT0, TWN, WXN, WYN, DTN, IERR)


Was this a bug? It looks like the winds were being read rather than ice concentration?

Actually - I see you mentioned a bug in the homogenous ice generation in your currents, I assume it is that.

Yes, that is the bug I mentioned.

JessicaMeixner-NOAA · 2021-05-13T01:35:14Z

Running tests from this PR + PR#373 all completed without errors!!!!

The comparison script is still running. @aliabdolali they will be in /scratch2/NCEPDEV/climate/Jessica.Meixner/PR_WW3/PR285/WW303/regtests I compared with the develop branch (one commit ago) not the results from #373. Results still need to be checked, but at least all the tests are succeeding now!

aliabdolali · 2021-05-13T02:42:17Z

@JessicaMeixner-NOAA Thanks for digging into the problem and provide the fix.
Here, I looked at the matrix.comp outputs. The differences are due to changes in log files and known issues. Also, ww3_ts3 is changes and differences are expected.
MatrixDiff_UKMET_coupling.zip

ukmo-ccbunney · 2021-05-13T08:14:36Z

Top job @JessicaMeixner-NOAA! 🥇
Thanks for your help in getting to the bottom of this.

ukmo-ccbunney · 2021-05-14T08:47:08Z

@aliabdolali, @JessicaMeixner-NOAA , @ukmo-juan-castillo - I am going to merge in develop to pull in fix #373 and then re-run the regtests for this PR.

aliabdolali · 2021-05-14T15:41:35Z

I have done the tests successfully, after merging develop to this PR. Thanks to Jessica's bug fix.
See the matrix.comp here:
DIFF_coupling.zip

For all the cases, the log.* files are different due to the additional fields introduced in the table.
The restart files are different for all the cases, but no changes in the gridded and point output, except for:

mww3_test_03
ww3_tp2.18 
ww3_tp2.10
mww3_test_07(mod_def)
mww3_test_04(mod_def)

For the following, I need your inputs @ukmo-ccbunney @ukmo-juan-castillo

* test case: ww3_tic1.4; test run: ./work_IC1IS2_1000
*********************************************************
     out_grd.ww3 differ (binary)
* test case: ww3_tic1.4; test run: ./work_IC2IS2_IC2d
*********************************************************
     out_grd.ww3 differ (binary)
* test case: ww3_tic1.4; test run: ./work_IC0IS2_1000
*********************************************************
     out_grd.ww3 differ (binary)
* test case: ww3_tic1.4; test run: ./work_IC2IS2_IC2b
*********************************************************
     out_grd.ww3 differ (binary)
* test case: ww3_tic2.3; test run: ./work_IC2IS2dissip
*********************************************************
     out_grd.ww3 differ (binary)
* test case: ww3_tic2.3; test run: ./work_IC2IS2scat
*********************************************************
     out_grd.ww3 differ (binary)
* test case: ww3_tic2.3; test run: ./work_IC2IS2creep
*********************************************************
     out_grd.ww3 differ (binary)
* test case: ww3_tp2.14; test run: ./work_OASACM5
*********************************************************
     output.ww3 differ.
* test case: ww3_tp2.14; test run: ./work_OASACM6
*********************************************************
     output.ww3 differ.
* test case: ww3_tp2.14; test run: ./work_OASACM4
*********************************************************
     output.ww3 differ.
* test case: ww3_tp2.14; test run: ./work_OASACM
*********************************************************
     output.ww3 differ.
* test case: ww3_tp2.14; test run: ./work_OASICM
*********************************************************
     output.ww3 differ.
     out_grd.ww3 differ (binary)
* test case: ww3_tp2.14; test run: ./work_OASOCM
*********************************************************
     output.ww3 differ.
* test case: ww3_tp2.14; test run: ./work_OASACM2
*********************************************************
     output.ww3 differ.

aliabdolali · 2021-05-14T21:04:51Z

regtests/ww3_tp2.15/input_rho/ww3_shel.inp

@@ -0,0 +1,48 @@
+$ -------------------------------------------------------------------- $


@ukmo-juan-castillo Could you add in the ww3_tp2.15/info about the added tests imput_rho, so later we can use it to check this feature.
A related question, does this new regtest check the interpolation of the new fields?

Added some information about the new test, which checks the interpolation of the new fields. Please let me know if it is clear.

…into fb_coupling_fields

JessicaMeixner-NOAA · 2021-05-17T15:10:19Z

@aliabdolali was there any explanation about the differences in out_grd in the tests you mention? It looks like all of the information computed from the out_grd files are the same, but still seems odd that these tests have diffs in the out_grd.

aliabdolali · 2021-05-17T16:21:11Z

@aliabdolali was there any explanation about the differences in out_grd in the tests you mention? It looks like all of the information computed from the out_grd files are the same, but still seems odd that these tests have diffs in the out_grd.

While I am waiting for @ukmo-juan-castillo's feedback on this, I'll rerun these tests again.

JessicaMeixner-NOAA · 2021-05-17T16:35:53Z

@aliabdolali thanks for the update. I'll wait on the new results and/or reply from @ukmo-juan-castillo to complete my code review.

aliabdolali · 2021-05-17T19:58:38Z

An update from my side:
output.ww3 are just stdout and they are expected, but I need some clarification for the gridded outputs for the following tests (i reran them again):

* test case: ww3_tic1.4; test run: ./work_IC1IS2_1000
*********************************************************
     out_grd.ww3 differ (binary)
* test case: ww3_tic1.4; test run: ./work_IC2IS2_IC2d
*********************************************************
     out_grd.ww3 differ (binary)
* test case: ww3_tic1.4; test run: ./work_IC0IS2_1000
*********************************************************
     out_grd.ww3 differ (binary)
* test case: ww3_tic1.4; test run: ./work_IC2IS2_IC2b
*********************************************************
     out_grd.ww3 differ (binary)
* test case: ww3_tic2.3; test run: ./work_IC2IS2dissip
*********************************************************
     out_grd.ww3 differ (binary)
* test case: ww3_tic2.3; test run: ./work_IC2IS2scat
*********************************************************
     out_grd.ww3 differ (binary)
* test case: ww3_tic2.3; test run: ./work_IC2IS2creep
*********************************************************
     out_grd.ww3 differ (binary)
* test case: ww3_tp2.14; test run: ./work_OASICM
*********************************************************
     out_grd.ww3 differ (binary)

ukmo-juan-castillo · 2021-05-18T09:26:14Z

Sorry for the delay, I have been quite busy this days.

I believe the explanation for the differences in out_grd.ww3 comes from the lines:

https://github.com/ukmo-waves/WW3/blob/0af9e032152eaa841cac723691c15bca39201b3f/model/ftn/w3iogomd.ftn#L2641-L2645

This means the the flags FLOGRD are written to the file. The branch adds new output fields, and changes the flag number in FLOGRD for the fields D50, IC1, and IC5. Therefore, any time we write these fields to the output the FLOGRD flags will be different, although the behaviour of the program will be the same. If you use the command cmp in linux you should be able to see that the differences are just the order of some bits, corresponding to the logical variable FLOGRD.

aliabdolali · 2021-05-18T12:55:11Z

Sorry for the delay, I have been quite busy this days.

I believe the explanation for the differences in out_grd.ww3 comes from the lines:

https://github.com/ukmo-waves/WW3/blob/0af9e032152eaa841cac723691c15bca39201b3f/model/ftn/w3iogomd.ftn#L2641-L2645

This means the the flags FLOGRD are written to the file. The branch adds new output fields, and changes the flag number in FLOGRD for the fields D50, IC1, and IC5. Therefore, any time we write these fields to the output the FLOGRD flags will be different, although the behaviour of the program will be the same. If you use the command cmp in linux you should be able to see that the differences are just the order of some bits, corresponding to the logical variable FLOGRD.

Thanks for the explanation. My only concern is that it limits our matrix.comp functionality and reports these tests different.

ukmo-ccbunney · 2021-05-18T15:18:07Z

@aliabdolali @JessicaMeixner-NOAA
I've rerun the regtets for this PR and I get the following expected differences (ignoring the restart and log file differences), as explained by the new FLOGRD output to out_grd.ww3

* test case: ww3_tic1.4; test run: ./work_IC2IS2_IC2b
     out_grd.ww3 differ (binary)
* test case: ww3_tic1.4; test run: ./work_IC0IS2_1000
     out_grd.ww3 differ (binary)
* test case: ww3_tic1.4; test run: ./work_IC1IS2_1000
     out_grd.ww3 differ (binary)
* test case: ww3_tic1.4; test run: ./work_IC2IS2_IC2d
     out_grd.ww3 differ (binary)
* test case: ww3_tic2.3; test run: ./work_IC2IS2creep
     out_grd.ww3 differ (binary)
* test case: ww3_tic2.3; test run: ./work_IC2IS2dissip
     out_grd.ww3 differ (binary)
* test case: ww3_tic2.3; test run: ./work_IC2IS2scat
     out_grd.ww3 differ (binary)
* test case: ww3_tp2.14; test run: ./work_OASICM
     out_grd.ww3 differ (binary)

However, I also got lots of differences in the ww3_ts3 set of regtets, so I a re-running those and will check again.

JessicaMeixner-NOAA · 2021-05-18T15:22:30Z

@ukmo-ccbunney I thought the ww3_ts3 diffs were because of some bug fix?

ukmo-ccbunney · 2021-05-18T15:23:41Z

@aliabdolali @JessicaMeixner-NOAA
Ah - the ww3_ts3 differences are because we fixed the ww3_shel.inp file to correctly read in the homogenous currents (previously the input flag was set to false).

I'm happy with the matrix differences in that case.

ukmo-ccbunney · 2021-05-18T15:27:12Z

@ukmo-ccbunney I thought the ww3_ts3 diffs were because of some bug fix?

That fix is already in develop and this branch so should not show as a difference.

This was an existing problem with the shel input file. It got fixed when Ali, Jaun and I were trying to track down the bug that you eventually fixed.

ukmo-juan-castillo · 2021-05-18T15:55:22Z

As requested, a comment can be added to the merge, such as:

This branch adds new fields that can be sent (mean wave number, mean wave period, total ocean wind stress) and received (air density, wind stress) to/from other models via OASIS coupling, which will also be available in the model output. The fields that can be received via coupling can also be read from an external file, and using an homogeneous ice fraction is now activated when using '.inp' input files, as it was already possible to do it using '.nml' input files. In order to add this functionality it was necessary to modify the format of the ww3_shel.inp and ww3_multi.inp files, so that new flags controlling if these fields are used can be read. Furthermore, the format of the ww3_outf.inp file must be modified to accommodate the new output fields available.

If no air density is read from file or via coupling, the default constant value is used. The infrastructure to read and use the wind stress is already in place, but the actual use of wind stress to drive the model is not implemented yet and it will be the aim of issue #337

aliabdolali · 2021-05-18T17:00:32Z

Hi,
I checked the tests with differences in out_grd.ww3 and reran it again and compared with the existing tests, they are identical. So we are good to go.


**********************************************************************
************************ identical cases *****************************
**********************************************************************
ww3_tic1.4/./work_IC1IS2_1000
ww3_tic1.4/./work_IC2IS2_IC2d
ww3_tic1.4/./work_IC0IS2_1000
ww3_tic1.4/./work_IC2IS2_IC2b
ww3_tic2.3/./work_IC2IS2dissip
ww3_tic2.3/./work_IC2IS2scat
ww3_tic2.3/./work_IC2IS2creep
ww3_tp2.14/./work_OASACM5
ww3_tp2.14/./work_OASACM6
ww3_tp2.14/./work_OASACM4
ww3_tp2.14/./work_OASACM3
ww3_tp2.14/./work_OASACM
ww3_tp2.14/./work_OASICM
ww3_tp2.14/./work_OASOCM
ww3_tp2.14/./work_OASACM2

JessicaMeixner-NOAA

@ukmo-juan-castillo thanks for helping us work through all the issues. The new features look good

ukmo-juan-castillo and others added 15 commits October 5, 2020 17:07

First set of changes, still testing

c35acf6

Some fixes

69259ef

Further changes mainly fixing vector boundaries

4c272cc

Main changes

01177e4

Most changes already present, only need to write in output and restart

0166c0d

Further changes - activate coupling

f5be40a

Bug fixes to make the code compile

dd1913f

Further changes to run regtests

3677987

Minor fixes for output, manual, and switches

5c9974d

Merge branch 'develop' into fb_coupling_fields

f9cfcbb

Small fix to comments in model input

b72a330

Merge branch 'develop' into fb_coupling_fields

00d9fbf

Remove some lines that had to be removed but were overlooked

1358ea3

ukmo-juan-castillo added 2 commits January 6, 2021 17:21

Substitute ATTX and ATXX switches by WNTX and WNXX; add comments

656eb5e

regarding last modification date

Change in file missed in latest commit

4a21c10

ukmo-juan-castillo requested a review from ukmo-ccbunney January 6, 2021 17:27

ukmo-juan-castillo marked this pull request as draft January 6, 2021 17:28

JessicaMeixner-NOAA reviewed Jan 6, 2021

View reviewed changes

model/ftn/w3iogomd.ftn Show resolved Hide resolved

JessicaMeixner-NOAA reviewed Jan 6, 2021

View reviewed changes

model/ftn/w3iorsmd.ftn Show resolved Hide resolved

JessicaMeixner-NOAA reviewed Jan 6, 2021

View reviewed changes

model/ftn/w3fld1md.ftn Show resolved Hide resolved

ukmo-juan-castillo and others added 2 commits January 7, 2021 14:33

Merge branch 'develop' into fb_coupling_fields

a157dc9

Minor changes fixing version change

73ea7e1

ukmo-ccbunney suggested changes Jan 12, 2021

View reviewed changes

Small fixes after reviewer's comments and to fix regtest

3dad097

JessicaMeixner-NOAA mentioned this pull request May 13, 2021

Update WW3 with fb_coupling_fields; add cice fix for zlvs ufs-community/ufs-weather-model#573

Merged

13 tasks

Merge branch 'NOAA-EMC:develop' into fb_coupling_fields

6b53f1a

aliabdolali reviewed May 14, 2021

View reviewed changes

ukmo-juan-castillo added 2 commits May 17, 2021 09:06

Added comment to the new regtests implemented in ww3_tp2.15

1e28c49

Merge branch 'fb_coupling_fields' of https://github.com/ukmo-waves/WW3 …

0af9e03

…into fb_coupling_fields

aliabdolali requested review from JessicaMeixner-NOAA and ukmo-ccbunney May 17, 2021 13:06

JessicaMeixner-NOAA approved these changes May 18, 2021

View reviewed changes

aliabdolali approved these changes May 18, 2021

View reviewed changes

aliabdolali merged commit 482c500 into NOAA-EMC:develop May 18, 2021

ukmo-ccbunney mentioned this pull request May 21, 2021

ACTION array allocation too small #378

Closed

ukmo-ccbunney mentioned this pull request Jul 15, 2021

Bugfix homogeneous ice condition in ww3_shel #235

Closed

ukmo-juan-castillo deleted the fb_coupling_fields branch September 29, 2022 10:38

		@@ -0,0 +1,48 @@
		$ -------------------------------------------------------------------- $

Fb coupling fields #285

Fb coupling fields #285

Conversation

ukmo-juan-castillo commented Jan 6, 2021

ukmo-juan-castillo commented Jan 6, 2021 • edited Loading

ukmo-juan-castillo commented Jan 6, 2021

ukmo-juan-castillo commented Jan 6, 2021 • edited Loading

ukmo-juan-castillo commented Jan 6, 2021 • edited Loading

ukmo-ccbunney left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ukmo-ccbunney Jan 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ukmo-ccbunney Jan 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JessicaMeixner-NOAA commented May 13, 2021

aliabdolali commented May 13, 2021

ukmo-ccbunney commented May 13, 2021

ukmo-ccbunney commented May 14, 2021

aliabdolali commented May 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JessicaMeixner-NOAA commented May 17, 2021

aliabdolali commented May 17, 2021

JessicaMeixner-NOAA commented May 17, 2021

aliabdolali commented May 17, 2021

ukmo-juan-castillo commented May 18, 2021

aliabdolali commented May 18, 2021

ukmo-ccbunney commented May 18, 2021 • edited Loading

JessicaMeixner-NOAA commented May 18, 2021

ukmo-ccbunney commented May 18, 2021

ukmo-ccbunney commented May 18, 2021 • edited Loading

ukmo-juan-castillo commented May 18, 2021

aliabdolali commented May 18, 2021

JessicaMeixner-NOAA left a comment

Choose a reason for hiding this comment

ukmo-juan-castillo commented Jan 6, 2021 •

edited

Loading

ukmo-juan-castillo commented Jan 6, 2021 •

edited

Loading

ukmo-juan-castillo commented Jan 6, 2021 •

edited

Loading

ukmo-ccbunney Jan 29, 2021 •

edited

Loading

ukmo-ccbunney Jan 12, 2021 •

edited

Loading

aliabdolali commented May 14, 2021 •

edited

Loading

ukmo-ccbunney commented May 18, 2021 •

edited

Loading

ukmo-ccbunney commented May 18, 2021 •

edited

Loading