Fix zero boundary and orbital velocity issue in the cap #1402

danishyo · 2025-04-04T20:19:46Z

Original issue can be found at oceanmodeling/ufs-weather-model#145 (comment)
It cause zero values along the boundary for exchange variables (like SXX) from cap.
Also, original code will derive weird orbital velocity (looks like domain-decomposition map, like the following figure)

@uturuncoglu , and @janahaddad are aware of these issues.
@DeniseWorthen , could you have a look on this fix? Thx!

DeniseWorthen · 2025-04-04T20:42:02Z

model/src/wav_import_export.F90

        ix  = mapsf(isea,1)
        iy  = mapsf(isea,2)
-        if (mapsta(iy,ix) == 1) then
+        if (mapsta(iy,ix) /= 0) then


Please don't change anything w/in the CESMCOUPLED ifdefs unless you're testing in CESM and know that the change is acceptable.

Got it! Thanks for the notice!

@danishyo can you make sure that this requested change is reflected in this PR?

@sbanihash I didn't intend to change CESM part,
only like to change exchange fields to SCHISM, like Hs, orbital, stoke's drift.... ,
please follow @DeniseWorthen suggestion, skip change within CESMCOUPLED .

@danishyo since this is your PR, what you will need to do is make these changes, commit and push them to your PR branch (which seems to be the dev/ufs-weather-model-branch in your fork) and then I will move forward with the review and testing for the PR. Thank you in advance.

@sbanihash Thanks for the explanation, I already revert the change within CESM block and push.

DeniseWorthen · 2025-04-13T14:06:33Z

@danishyo The fact that you see the decomposition reflected on the boundary gives me pause. I would expect if it were a simple issue of mapsta being incorrect, you might see garbage (or zero), but you're seeing something that looks like a decomposition.

I don't know how much debugging you've done for this issue in WW3 itself, but I reminded of this issue I found when adding the unstructured mesh to the cap

ufs-community/ufs-weather-model#1556 (comment)

In that case, the exported field was corrupted on the last PE, but only on the restart frequency.

I suspect that something more fundamental may be occurring inside of WW3 it it is manifesting itself in your case.

danishyo · 2025-04-14T15:41:06Z

@danishyo The fact that you see the decomposition reflected on the boundary gives me pause. I would expect if it were a simple issue of mapsta being incorrect, you might see garbage (or zero), but you're seeing something that looks like a decomposition.

I don't know how much debugging you've done for this issue in WW3 itself, but I reminded of this issue I found when adding the unstructured mesh to the cap

ufs-community/ufs-weather-model#1556 (comment)

In that case, the exported field was corrupted on the last PE, but only on the restart frequency.

I suspect that something more fundamental may be occurring inside of WW3 it it is manifesting itself in your case.

Hi @DeniseWorthen
I believe this is only related the code to derive orbital. I compare output for both WWM & WW3 (with update PR code), they are similar like the following figure.

Anyway, I will revert change within CESM block then push.

DeniseWorthen · 2025-04-14T15:51:37Z

@danishyo For the current UFS cases using the unstructured mesh, we do not export some of the fields which you are using in the coastal app. So I never did any debugging of possible issues inside of WW3 w/ those fields.

I don't doubt that you see that your fix resolves the issue---but I'm not 100% sure that the issue you are seeing isn't from something being done incorrectly inside of WW3. How else to explain that what you see is evidence of the decomposition of WW3 in the exported fields?

danishyo · 2025-04-14T17:24:37Z

@danishyo For the current UFS cases using the unstructured mesh, we do not export some of the fields which you are using in the coastal app. So I never did any debugging of possible issues inside of WW3 w/ those fields.

I don't doubt that you see that your fix resolves the issue---but I'm not 100% sure that the issue you are seeing isn't from something being done incorrectly inside of WW3. How else to explain that what you see is evidence of the decomposition of WW3 in the exported fields?

@DeniseWorthen I checked all exchange fields between SCHISM & WW3, orbital velocity is the only one showing this, others like Hs, Tm01... are not. So I just guessing this may not related.

DeniseWorthen · 2025-04-14T17:35:35Z

model/src/wav_import_export.F90

      if ( firstCall ) then
-        if(( runtype == 'initial'  .and.     mapsta(iy,ix)  == 1 ) .or. &
-             ( runtype == 'continue' .and. abs(mapsta(iy,ix)) == 1 )) then
+        if(( runtype == 'initial'  .and.     mapsta(iy,ix)  /= 0 ) .or. &


@danishyo Did you intend to change this block? I don't remember that coastal app exports Z0 and this is w/in code that UFS global mode uses.

No, I didn't, I will revert this one as well.

DeniseWorthen · 2025-04-14T17:43:40Z

@danishyo For the current UFS cases using the unstructured mesh, we do not export some of the fields which you are using in the coastal app. So I never did any debugging of possible issues inside of WW3 w/ those fields.
I don't doubt that you see that your fix resolves the issue---but I'm not 100% sure that the issue you are seeing isn't from something being done incorrectly inside of WW3. How else to explain that what you see is evidence of the decomposition of WW3 in the exported fields?

@DeniseWorthen I checked all exchange fields between SCHISM & WW3, orbital velocity is the only one showing this, others like Hs, Tm01... are not. So I just guessing this may not related.

I've had enough experience adding features and debugging in WW3 that when I see something like a decomposition field being expressed in the output field, it is difficult for me not be suspicious. The fact that you're seeing it in some things and not others doesn't really convince me.

However, UFS is not making use of these fields that coastal is using currently, so if you restrict your changes to only coastal-app exported fields, I'm ok with your changes. I just would not be at all surprised to find something deeper in WW3 is actually causing a decomposition to appear in an exported field.

danishyo · 2025-04-14T17:46:33Z

Thanks for the explanation, I just revert the changes and keep changes with necessary fields.

JessicaMeixner-NOAA

I'm wondering if these should be if mapsta(iy,ix) > 0 as negative values indicate that a point is currently taken out of computation. https://github.com/NOAA-EMC/WW3/blob/develop/model/src/w3gridmd.F90#L219-L247

Also I'm not sure what prompted the changes in the calcstokes computations?

JessicaMeixner-NOAA · 2025-04-25T21:01:04Z

model/src/wav_import_export.F90

          end do
-          kd = max ( 0.001 , wn(ik,isea) * dw(isea) )
-          if (kd .lt. 6.0) then
+          kd = max ( 1.0e-7 , wn(ik,isea) * dw(isea) ) !0.001


Why are there changes to this calculation? This appears different than the mapsta issue updated elsewhere but I don't see anything about it in the PR description.

I borrow routine coefficients from https://github.com/NOAA-EMC/WW3/blob/develop/model/src/wmesmfmd.F90#L7315
For mapsta, I am OK to change ">0" if this includes ocean and boundary points.

The code for the ussco matches (or should) what is currently in w3iogomd.F90

@danishyo - Can you let us know how your testing goes for ">0" this should be sufficient.

I would think this calculation should match what is in w3iogo. I'd have to look at the differences between that and wmesmf as to why that would have been diverged. Let me know if you need help investigating this further @danishyo or if you will take the lead on that.

@danishyo were you ever able to run those tests?

@danishyo were you ever able to run those tests?

I didn't do "mapsta>0" test, but base on info from manual, I believe this should fix the issue as well.

Let us know when you've been able to run those tests and confirm if that works or not.

We also have outstanding issues about this part of the code change and why it's diverged, etc.

This PR is waiting on those updates to move forward.

Fix zero boundary and orbital velocity issue in the cap

2241e43

danishyo mentioned this pull request Apr 4, 2025

Variables passing through CMEPS to SCHISM oceanmodeling/ufs-weather-model#145

Open

DeniseWorthen reviewed Apr 4, 2025

View reviewed changes

janahaddad moved this to In Progress in ufs-coastal project Apr 11, 2025

janahaddad added this to ufs-coastal project Apr 11, 2025

Revert mapsta if condition within CESM block

72b634a

DeniseWorthen reviewed Apr 14, 2025

View reviewed changes

Revert changes for unused fields

444aa2b

JessicaMeixner-NOAA reviewed Apr 25, 2025

View reviewed changes

Fix zero boundary and orbital velocity issue in the cap #1402

Are you sure you want to change the base?

Fix zero boundary and orbital velocity issue in the cap #1402

Uh oh!

Conversation

danishyo commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DeniseWorthen commented Apr 13, 2025

Uh oh!

danishyo commented Apr 14, 2025

Uh oh!

DeniseWorthen commented Apr 14, 2025

Uh oh!

danishyo commented Apr 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DeniseWorthen commented Apr 14, 2025

Uh oh!

danishyo commented Apr 14, 2025

Uh oh!

JessicaMeixner-NOAA left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

danishyo commented Apr 4, 2025 •

edited

Loading