Skip to content

Bugfix rotbd#1

Open
Carsten-Hansen wants to merge 4 commits into
FCOO:fb_xsmffrom
Carsten-Hansen:bf_rotbd
Open

Bugfix rotbd#1
Carsten-Hansen wants to merge 4 commits into
FCOO:fb_xsmffrom
Carsten-Hansen:bf_rotbd

Conversation

@Carsten-Hansen
Copy link
Copy Markdown

@Carsten-Hansen Carsten-Hansen commented Apr 24, 2026

Pull Request Summary

This bugfix corrects the erroneous advection across multi-grid boundaries when using rotated grids (switch RTD) with ww3_multi

This bug fix is intended to be pulled by @MadsBruunPoulsen into the FCOO/WW3 branch fb_xsmf.

It is a tentative solution to the issue NOAA-EMC#1579 by @CarstenHansen, following the suggestion given there.

Description

The bug is described in detail in NOAA-EMC#1579. The solution provides the expected correct grid boundary transfer under ww3_multi, e.g. as illustrated in Fig. 2.

A flag is declared global in w3_iobcmd.F90 where it is set to 'TRUE' only when reading boundary conditions from file (nest.ww3), and used in w3updtmd.F90 to indicate when rotation must be done (as suggested in NOAA-EMC#1579 (comment)). Setting this flag is not a property of the individual grid, but distinguish if if boundary conditions are either read from file or transferred by the ww3_multi mechanism.

Issue(s) addressed

Fixes NOAA-EMC#1579 (comment)

Commit Message

Correct the erroneous advection across multi-grid boundaries when using rotated grids (switch RTD) with ww3_multi.
Co-author: @MadsBrunPoulsen.

Check list

Testing

  • How were these changes tested? A suggested new regression test is included as rotated grid set (grdset_r), an extension to the existing advection regtest mww3_test02.
  • Are the changes covered by regression tests? (If not, why? Do new tests need to be added?) I suggest adding grdset_r to mww3_test02. Suggested additions to the info file is provided in info_rtd. The program ‎regtests/bin/run_cmake_test is modified to be able to create the initial files for the rotated grids.
  • Have the matrix regression tests been run (if yes, please note HPC and compiler)? NO
  • Please indicate the expected changes in the regression test output, (Note the list of known non-identical tests.)
  • Please provide the summary output of matrix.comp (matrix.Diff.txt, matrixCompFull.txt and matrixCompSummary.txt):

@Carsten-Hansen Carsten-Hansen changed the base branch from develop to fb_xsmf April 28, 2026 09:29
Comment thread model/src/w3iobcmd.F90
Comment thread model/src/w3updtmd.F90 Outdated
!! Use rotation angle and action conversion sub. JGLi12Jun2012
USE W3GDATMD, ONLY: NK, NTH, NSPEC, AnglD, PoLat
USE W3SERVMD, ONLY: W3ACTURN
!! BCTURN==.TRUE. only when calling W3UBPT from W3IOBC,
Copy link
Copy Markdown

@MadsBruunPoulsen MadsBruunPoulsen May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W3IOBC is not calling W3UBPT, as far as I can tell? Isn't it W3WAVE?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think you are right. I now pushed a correction.

Comment thread model/src/w3iobcmd.F90 Outdated
IDSTRBC = 'WAVEWATCH III BOUNDARY DATA FILE'
!/
#ifdef W3_RTD
! Logical to tell subroutine W3UBPT if inbound boundary conditions are to be roteted
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spelling mistake: "roteted" --> "rotated"

Copy link
Copy Markdown
Author

@Carsten-Hansen Carsten-Hansen May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this. To be committed and pushed. My fingers often makes this wrong auto-correction.

Comment thread model/src/w3updtmd.F90
USE W3SERVMD, ONLY: W3ACTURN
!! BCTURN==.TRUE. only when calling W3UBPT from W3IOBC,
!! This is in order *not* to turn 2-way nested bdy data under ww3_multi
USE W3IOBCMD, ONLY: BCTURN
Copy link
Copy Markdown

@MadsBruunPoulsen MadsBruunPoulsen May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make more sense to control this behavior from W3WAVE instead of loading this variable from W3IOBCMD? I guess that W3WAVE only calls W3IOBC if it needs to write a nest.ww3 file, so one could also introduce the logical in W3WAVE instead of W3IOBCMD. This logical could then work as an input when calling W3UBPT in W3WAVE and then you would not have to reset BCTURN=FALSE at the bottom. With my limited insights into WW3 architechure I would think that this is a more readable edit.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds reasonable. Let us check the logic of such change when we discuss the code.

Copy link
Copy Markdown

@MadsBruunPoulsen MadsBruunPoulsen May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHA to understand where w3iobc is called in w3wave and determine how straightforward it is to modify to control behavior from there. Note that if controlled from w3wave it is not necessary to keep modifications to w3iobcmd.F90 (assumption).

Comment thread regtests/mww3_test_02/input/ww3_grid_rtd_fine.inp
Comment thread regtests/mww3_test_02/input/ww3_grid_rtd_fine.inp
$ fp sip thm ncos xm six ym siy hmax
0.10 0.0001 225. 100 -3.15 -0.675 -3.15 0.675 10.0
$ -0.225*3 0.225*3
$ 0.10 0.0001 225. 100 600.E3 -75.E3 600.E3 75.E3 10.0
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove these commented lines?

Copy link
Copy Markdown
Author

@Carsten-Hansen Carsten-Hansen May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! To be removed in the next commit.

@@ -0,0 +1,5 @@
&OUTS E3D=1, TH1MF=1, STH1MF=1, TH2MF=1, STH2MF=1 /
&PRO2 DTIME = 0. /
&PRO3 WDTHCG = 0., WDTHTH = 0. /
Copy link
Copy Markdown

@MadsBruunPoulsen MadsBruunPoulsen May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are settings for both PRO2 and PRO3 mentioned as the regtest can be run with different switches that both involve PRO2 and PRO3? The info_fcoo only mentions switch_PR3_UQ_MPI_RTD.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be a reason, but I just copied the contents of namelisis_fine.nml without thinking about it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider to outcomment

Comment thread regtests/mww3_test_02/input/ww3_grid_rtd_fine.inp
@@ -0,0 +1 @@
1.35 1.35 'point_A'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remind me; point output coordinates are defined in the rotated grid, correct? So this coordinate is within the fine grid?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually forgot the answer to this. I think the answer is given in the User manual, or check the points specified for the FCOO GRL9 grid.

Comment thread regtests/mww3_test_02/input/ww3_grid_rtd_outer.inp
Comment thread regtests/mww3_test_02/input/ww3_multi_grdset_r.nml
Comment thread regtests/mww3_test_02/info_fcoo Outdated
# + grdset_b: outer + fine

# * Switch:
# + switch_PR3_UQ_MPI_RTD
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you miss to add this switch to the pull request?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, to be added in next commit

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now added, and pushed

Comment thread regtests/mww3_test_02/info_fcoo Outdated
Comment on lines +35 to +45
# Run the WW3 run_cmake_test script at FCOO
#
# # Load a build environment using the gfortran compiler:
#
# spack load netcdf-fortran%gcc
# spack load cmake%gcc
#
# # Run the WW3 run_cmake_test script from the directory containg the WW3 repo:
#
# ../ww3_resurser/run_regtests.sh -N -g rtd -m grdset_r -n 3 -s PR3_UQ_MPI_RTD \
# mww3_test_02
Copy link
Copy Markdown

@MadsBruunPoulsen MadsBruunPoulsen May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are moving away from Spack - let's discuss this tomorrow. Also, I don't think we want FCOO specific information in the pull request. All FCOO specific information must be put elsewhere, in my opinion. As the information is this file is more or less identical to what's written in info_rtd I suggest to delete this file.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. info_fcoo is to be removed in the next git commit

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You wrote a regtest that checks that behavior is as expected for ww3_multi with switch RTD. Are we missing any checks, e.g. that a ww3_multi model run with a regular pole is able to output boundary conditions to a destination grid that is rotated?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If ww3_multi writes boundary files not identical to the output with ww3_shel, that would be a separate issue.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHA: Missing to test that if outer domain has incoming boundary conditions (nest.ww3 defined) it is correctly rotated to rank 1 grid and that BCTURN is correctly set to false afterwards when nesting between outer and fine grid. That would test both cases of BCTURN.

@Carsten-Hansen
Copy link
Copy Markdown
Author

I am preparing an extension to the rotated grdset_r with a very coarse grid that reads boundary data from a file nest.ww3, which is first generated by bi-linear interpolation from three spectra at three points surrounding the (rotated-)southern grid edge. nest.ww3 is generated using the program ww3_bound, and the three spectra are copied from ChrisBunney's regtests/ww3_tr1/input_bndin/bnd{1,2,3}.spc. The spectra direction origin should be shifted 180 degree, as in Chris' case they are surrounding the (standard-)eastern boundary edge, and in my case the (standard-)western edge.

@@ -0,0 +1,5 @@
&OUTS E3D=1, TH1MF=1, STH1MF=1, TH2MF=1, STH2MF=1 /
&PRO2 DTIME = 0. /
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here as well

Comment on lines +736 to +741
if [ $grdstr ]
then
fileconf="${prog}_${grdstr}${gu}"
else
fileconf="${prog}${gu}"
fi
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some cases this does not exist: ww3_strt_${grdstr}${gu} while the version without ${grdset} does exist. Please consider.

Comment on lines +35 to +36
# ./bin/run_cmake_test -N -g rtd -m grdset_r -n 3 -p mpirun \ #
# -s PR3_UQ_MPI_RTD -w work_bp_PR3_MPI_RTD ../model mww3_test_02 #
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am able to run the regtest but not with the -N (use .mnl files) flag. It works with .inp files. The output looks strange, however, and not as presented in your issue post on NOAAs GitHub account. Are you able to run the regtest with .nml files successfully?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Under switch RTD, the two-way nesting is corrupted when running ww3_multi

3 participants