Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix barotropic streamfunction calculation #1069

Merged
merged 10 commits into from
Mar 7, 2025
Merged

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Mar 3, 2025

Instead of computing the BSF using a least-squares solve based on the transport between vertices, the new approach inverts a Poisson equation for the BSF based on the (vertically integrated) vorticity.

I have proven that this produces more accurate results. The vertically integrated vorticity computed from the streamfunciton is within machine precision of the original vertically integrated vorticity. The velocity produced by the streamfunciton has errors that are on the order of 1e-3 compared with the original vertically integrated velocity (likely caused by the field not being perfectly divergence free).

Checklist

  • User's Guide has been updated
  • Documentation has been built locally and changes look as expected
  • Testing comment in the PR documents testing used to verify the changes

Fixes #1068

@xylar xylar added the bug label Mar 3, 2025
@xylar xylar requested review from maltrud and milenaveneziani March 3, 2025 14:41
@xylar xylar self-assigned this Mar 3, 2025
@xylar
Copy link
Collaborator Author

xylar commented Mar 3, 2025

I wrote out some NetCDF and VTK debugging output with the following code snippet:

        v0 = vertices_on_edge.isel(TWO=0)
        v1 = vertices_on_edge.isel(TWO=1)
        valid = np.logical_and(v0 >= 0, v1 >= 0)
        transport_check = -valid * (bsf_vertex.isel(nVertices=v1) -
                                    bsf_vertex.isel(nVertices=v0))

        velocity_check = 1e6 * transport_check / dv_edge

        vorticity_check = self._compute_vert_integ_vorticity(
            ds_mesh, velocity_check, edge_sign_on_vertex)

        ds_out = xr.Dataset()
        ds_out['barotropicStreamfunction'] = bsf_vertex
        ds_out['verticallyIntegratedVelocity'] = vert_integ_velocity
        ds_out['verticallyIntegratedVorticity'] = vert_integ_vorticity
        ds_out['transportCheck'] = transport_check
        ds_out['verticallyIntegratedVelocityCheck'] = velocity_check
        ds_out['verticallyIntegratedVorticityCheck'] = vorticity_check
        out_filename = f'bsf_{self.subtaskName}.nc'
        ds_out.to_netcdf(out_filename)

        extract_vtk(out_filename, mesh_filename=self.restartFileName,
                    out_dir=f'vtk_bsf_{self.subtaskName}',
                    dimension_list=['vertexDegree=:'],
                    ignore_time=True)

The results indicate that the vertically integrated vorticity is almost identical if computed from the original velocity and from the streamfunction:
vert_integ_vort
vert_integ_vort_diff

The same is not quite true of the vertically integrated normal velocity, which has differences on the order of 1e-3 of the maximum velocity:
vert_integ_vel
vert_integ_vel_diff

This deserves further investigation because it seems like somewhat too large an error to be accounted for by the divergent part of the vertically integrated velocity, which should be very close to zero, especially in a 6-year time average.

@xylar
Copy link
Collaborator Author

xylar commented Mar 3, 2025

Testing

I ran this analysis on a 6-year climatology from a G-case with the SORRMr3 mesh. I had previously noticed that the streamfunction produced for this mesh didn't make sense, particularly in the FRIS area (see #1068). I looked at the transport computed from the BSF compared with the original transport, and they differed by and unacceptably large factors, indicating problems with the methodology even after the fix in #1063.

With the rewrite described above, I'm seeing the results in:
https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.xylar/analysis_testing/BSF/20250125.GMPAS-JRA1p5-DIB-PISMF.TL319_SOwISC12to30E3r3.chrysalis/ocean/index.html#antarctic_extended_bsf

image

image

image

After playing around with colorbars and colormaps quite a bit, I think this linear colormap with an unusual color sequence (the one we often use for resolution plots from Compass) is a better choice than the one from #1063. The fix here increases the maximum value of the BSF (in the middle of the Weddell Gyre) even more than previously, resulting in a completely saturated colormap in Antarctica if we stay with the symmetric-log approach. By effectively having the blue range of the colormap only used in Antarctica, we can have a second linear (divergent) segment that is appropriate for that region.

Thoughts are welcome.

@xylar
Copy link
Collaborator Author

xylar commented Mar 3, 2025

@maltrud and @milenaveneziani, it seems like I messed up again in #1063 and I'll need you to give this another look. Sorry I'm not doing my best work.

This new approach is based on a proper definition of the BSF:

$$\omega H = -\nabla^2 \psi$$

where $\omega H$ is the vertically integrated vorticity and $\nabla^2$ is the Laplacian on the dual mesh (appropriate for vertices).

@xylar
Copy link
Collaborator Author

xylar commented Mar 3, 2025

I want to solve for the divergent contribution to $v H$ and make sure the difference I plotted above is explained by it. If not, I still must be missing something.

@alicebarthel
Copy link
Contributor

Just my 2c but the global plot looks really good to me: good Drake Passage transport, well-defined fronts in Agulhas retroflection and Southern Ocean. I find the new colorbar to be easily readable, well done.

@xylar
Copy link
Collaborator Author

xylar commented Mar 3, 2025

Thanks so much, @alicebarthel!

@xylar xylar requested a review from irenavankova March 3, 2025 17:56
@xylar
Copy link
Collaborator Author

xylar commented Mar 3, 2025

@milenaveneziani
Copy link
Collaborator

@xylar : would you be OK with me taking a closer look at this towards the end of this week? My plate is full until at least Th morning, but I am definitely interested/invested in having a correct BSF calculation and thus help with this.

One quick comment I have is about the colorbar: I am personally using the cmocean 'curl' for BSF, but this one looks nice as well. My only suggestion is about flipping the colors. Positive values: orange and reds, negative values: greens and blues (similar to our color association with pos/neg anomalies --> reds/blues). What do you think?

@xylar
Copy link
Collaborator Author

xylar commented Mar 3, 2025

@milenaveneziani, I will wait as long as I can but we are getting close to a new unified release and I need this to be included.

I will see what I can do about the color map but the fact is that we have positive values that are twice as high as negative and green/blue takes up 2/3 of this colormap. So we would need to create a new colormap for what you request.

@xylar
Copy link
Collaborator Author

xylar commented Mar 4, 2025

@milenaveneziani, here's what I came up with for an alternative colormap:
https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.xylar/analysis_testing/BSF/20250125.GMPAS-JRA1p5-DIB-PISMF.TL319_SOwISC12to30E3r3.chrysalis/ocean/index.html

image
image
image
image

To my eyes, this is not as aesthetic as the one I used above but it does satisfy the desire to have warm colors for positive and cool for negative.

I also tried green instead of blue and purple instead of brown but none of these other combinations looked very good.

image

Feel free to play around yourself at https://sciviscolor.org/color-moves-app/ and suggest an alternative if you want to take the time. (But I do need to have this done soon for Unified 1.11.0.)

@xylar
Copy link
Collaborator Author

xylar commented Mar 4, 2025

I computed the vertically integrated divergence, and from that I computed the velocity potential. The vertically integrated velocity (VIV) should be the gradient of the potential plus the normal cross the gradient of the barotropic streamfunction. For whatever reason I'm not quite getting this -- the contribution from the velocity potential is actually larger than the difference between the VIV and the piece computed as k cross grad psi. I'd like to understand this better but I'm running out of time I can spare for this work. I think for now I'm content to know that k x grad psi does a very good job now of predicting the VIV field, and the residual is small.

@xylar
Copy link
Collaborator Author

xylar commented Mar 4, 2025

@maltrud, @milenaveneziani and @irenavankova, I've made some more changes to this branch since I asked you to review yesterday, so please take that into account when you review. Sorry for the moving target. I believe I'm done until I get further feedback.

@@ -1578,7 +1577,7 @@ seasons = ['ANN']
comparisonGrids = ['latlon', 'subpolar_north_atlantic']

# list of tuples(pairs) of depths (min, max) to integrate horizontal transport over
depthRanges = [(0.0, -10000.0), (0.0, -2000.0)]
depthRanges = [(10.0, -10000.0), (10.0, -2000.0)]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It turns out that zero is not a safe upper bound because this cuts off a few parts of the ocean where the SSH is more than 1 m high (where the middle of the 2-m top layer can be above zero).

@xylar xylar force-pushed the fix-bsf-again branch 3 times, most recently from c73596b to b02414a Compare March 4, 2025 14:56
@irenavankova
Copy link
Collaborator

I made the FRIS focused plots for the 3 versions of the calculations and they are below. I am not sure what to think about the large transport values in the cavity, and all along the coast of Antarctica - what does the nonzero value there imply for the interpretation of clockwise/counterclockwise circulation for positive/negative values?

Screenshot 2025-03-04 at 13 59 52

For comparison, here is a bsf plot from a related regional study of the region, that is easier to interpret:

https://agupubs.onlinelibrary.wiley.com/cms/asset/e4d064df-52a7-4b8f-ba21-2fe278b21c6f/jgrc23944-fig-0008-m.jpg

@xylar
Copy link
Collaborator Author

xylar commented Mar 5, 2025

Thanks for the plots, @irenavankova.

I am not sure what to think about the large transport values in the cavity, and all along the coast of Antarctica - what does the nonzero value there imply for the interpretation of clockwise/counterclockwise circulation for positive/negative values?

There is an arbitrary integration constant in the streamfunction. The only thing that has a physical meaning are differences or gradients in the stream function. So the over all high value (over 150 Sv at the Antarctic coast) tells you that we have that much transport between Antarctica and coastlines to the North (so presumably across Drake Passage). It isn't possible to interpret positive vs. negative signs of the streamfunction as positive vs. negative circulation unless you reference it to the value of the streamfunciton locally (e.g. at the grounding line of FRIS). Unfortunately, that's not possible to do globally.

@xylar
Copy link
Collaborator Author

xylar commented Mar 5, 2025

@irenavankova, following up, it probably makes sense to have a way to plot the streamfunction referenced to the mean value around Antarctica, rather than around the continents north of 45 south. I'll think about how to do that. I think that situation would require a different colormap, too.

comparisonFrisResolution = 10.
comparisonFrisResolution = 4.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@irenavankova, I think we should make the FRIS comparison grid higher resolution. It seems a little blocky in the plots I was looking at when it's 10 km. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That is fine, as long as there aren't any artifacts popping up from interpolating low res to high res (I have seen that in some field before but here it looks fine)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We always use linear interpolation in MPAS-Analysis and that should always be fine from coarse to fine. The main place it's noticeable is at the coastline (when it doesn't match cartopy) and it isn't a ton better with higher res. But it also improves the streamline a little bit (less jagged).

@@ -611,7 +611,78 @@ yLim = [-600., -5.]

# comparison grid(s) on which to plot analysis
comparisonGrids = ['latlon', 'arctic_extended', 'antarctic_extended',
'subpolar_north_atlantic']
'subpolar_north_atlantic', 'fris']
Copy link
Collaborator Author

@xylar xylar Mar 5, 2025

Choose a reason for hiding this comment

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

@irenavankova, I think we might as well add FRIS plots of the BSF to polar regions. Again, what do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure, I didn't realize that needed to be done explicitly for bsf

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, we don't add any of the projection plots implicitly anywhere. If there are other FRIS plots you want to have on by default, they need to be added as well (but not in this PR).

Copy link
Collaborator

@irenavankova irenavankova left a comment

Choose a reason for hiding this comment

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

This is looks great, I don't see any issues. Thanks for digging into the inconsistencies and fixing them.

@xylar
Copy link
Collaborator Author

xylar commented Mar 6, 2025

@irenavankova, great, thanks for the review!

@xylar
Copy link
Collaborator Author

xylar commented Mar 7, 2025

@milenaveneziani, please have a look when you have time. It seems the Unified release isn't that eminent -- E3SM-Diags and zstash are both still debugging issues.

Copy link
Collaborator

@milenaveneziani milenaveneziani left a comment

Choose a reason for hiding this comment

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

@xylar: this was a lot of work! Thanks for making these big changes: they make sense to me. I also went through the compute_streamfunction code and things look good to me. Great work!

@xylar
Copy link
Collaborator Author

xylar commented Mar 7, 2025

Here are the new plots:
image

image

@xylar xylar removed the request for review from maltrud March 7, 2025 17:49
@xylar
Copy link
Collaborator Author

xylar commented Mar 7, 2025

@maltrud, didn't hear from you so will assume you didn't have anything to add.

@xylar xylar merged commit f272854 into MPAS-Dev:develop Mar 7, 2025
9 of 10 checks passed
@xylar
Copy link
Collaborator Author

xylar commented Mar 7, 2025

@milenaveneziani, @alicebarthel and @irenavankova, thanks so much for the input!

@xylar xylar deleted the fix-bsf-again branch March 7, 2025 17:50
@milenaveneziani
Copy link
Collaborator

Thanks for posting these!
So, the changes also apply to the subpolar_north_atlantic plot (not just ArcticExtended)? They look great for that region!

@xylar
Copy link
Collaborator Author

xylar commented Mar 7, 2025

Yes, I added the same changes for both regions. But any further changes have to be made in both config sections if we want them to look the same.

@xylar
Copy link
Collaborator Author

xylar commented Mar 7, 2025

Woops! Looks like my last commit didn't get pushed. My Internet connection is bad.

@milenaveneziani
Copy link
Collaborator

I think the changes are perfect for the subpolar region. I will test some adjustment to the ArcticExtended region and get back to you, but for now it's great that you added the two sections in the polar_regions config file.

@xylar
Copy link
Collaborator Author

xylar commented Mar 7, 2025

Sounds good.

xylar added a commit to xylar/MPAS-Tools that referenced this pull request Mar 9, 2025
This merge is a port of:
MPAS-Dev/MPAS-Analysis#1069

Instead of computing the BSF using a least-squares solve based on the transport between vertices, the new approach inverts a Poisson equation for the BSF based on the (vertically integrated) vorticity.

Using a SORRM G-Case simulation, the vertically integrated vorticity computed from the streamfunciton is within machine precision of the original vertically integrated vorticity. The velocity produced by the streamfunciton has errors that are on the order of 1e-3 compared with the original vertically integrated velocity (likely caused by the field not being perfectly divergence free).
xylar added a commit to xylar/MPAS-Tools that referenced this pull request Mar 9, 2025
This merge is a port of:
MPAS-Dev/MPAS-Analysis#1069

Instead of computing the BSF using a least-squares solve based on the transport between vertices, the new approach inverts a Poisson equation for the BSF based on the (vertically integrated) vorticity.

Using a SORRM G-Case simulation, the vertically integrated vorticity computed from the streamfunciton is within machine precision of the original vertically integrated vorticity. The velocity produced by the streamfunciton has errors that are on the order of 1e-3 compared with the original vertically integrated velocity (likely caused by the field not being perfectly divergence free).
xylar added a commit to xylar/MPAS-Tools that referenced this pull request Mar 10, 2025
This merge is a port of:
MPAS-Dev/MPAS-Analysis#1069

Instead of computing the BSF using a least-squares solve based on the transport between vertices, the new approach inverts a Poisson equation for the BSF based on the (vertically integrated) vorticity.

Using a SORRM G-Case simulation, the vertically integrated vorticity computed from the streamfunciton is within machine precision of the original vertically integrated vorticity. The velocity produced by the streamfunciton has errors that are on the order of 1e-3 compared with the original vertically integrated velocity (likely caused by the field not being perfectly divergence free).
xylar added a commit to xylar/MPAS-Tools that referenced this pull request Mar 10, 2025
This merge is a port of:
MPAS-Dev/MPAS-Analysis#1069

Instead of computing the BSF using a least-squares solve based on the transport between vertices, the new approach inverts a Poisson equation for the BSF based on the (vertically integrated) vorticity.

Using a SORRM G-Case simulation, the vertically integrated vorticity computed from the streamfunciton is within machine precision of the original vertically integrated vorticity. The velocity produced by the streamfunciton has errors that are on the order of 1e-3 compared with the original vertically integrated velocity (likely caused by the field not being perfectly divergence free).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug likely still present in barotropic streamfunction
4 participants