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

If available, plot climatology of total melt rate #989

Merged

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Feb 27, 2024

Also plot constituents: the melt rate at the interface and the frazil flux below ice shelves.

This is to support changes in E3SM-Ocean-Discussion/E3SM#79

@xylar xylar marked this pull request as draft February 27, 2024 20:07
@xylar
Copy link
Collaborator Author

xylar commented Feb 27, 2024

I still need to make these changes to the timeSeriesAntarcticMelt analysis.

@xylar xylar force-pushed the switch-to-land-ice-freshwater-flux-total branch from 077052b to 519349f Compare February 28, 2024 08:44
@xylar xylar marked this pull request as ready for review February 28, 2024 09:00
@xylar
Copy link
Collaborator Author

xylar commented Feb 28, 2024

Testing

I ran analysis on an old simulation to ensure backwards compatibility:
https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.xylar/analysis/total_fwf/20240223.v3.LR.CRYO1850-DISMF.paolo.chrysalis/ocean/index.html#antmelttime

Then, I ran analysis on a new simulation with the branch from E3SM-Ocean-Discussion/E3SM#79:
https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.xylar/analysis/20240226.CRYO1850.ne30pg2_r05_IcoswISC30E3r5.include_frazil.chrysalis/yrs_5-5/ocean/index.html

Both worked as expected. No errors were raised with the old simulation and the plots for the missing variables simply do nothing.

For the analysis on the new simulation, comparison with observations only occurs for the total flux, not the interface melt rate (or the frazil flux).

I verified that the expected variable is used in the melt time series for each analysis run -- the old timeMonthly_avg_landIceFreshwaterFlux if necessary but the new timeMonthly_avg_landIceFreshwaterFluxTotal if available.

@xylar xylar requested a review from cbegeman February 28, 2024 09:05
@xylar
Copy link
Collaborator Author

xylar commented Feb 28, 2024

@irenavankova, do you want to have a look at this as well?

@xylar
Copy link
Collaborator Author

xylar commented Feb 28, 2024

Copy link
Collaborator

@cbegeman cbegeman left a comment

Choose a reason for hiding this comment

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

I just made a couple minor suggestions. Otherwise, I think this looks good!

mpas_analysis/ocean/time_series_antarctic_melt.py Outdated Show resolved Hide resolved
@irenavankova
Copy link
Contributor

For the Antarctic Melt Time Series, if the new variables aren't available, do you still want the header of the section to read
Total Melt Flux although it is only calculated from interface melt flux? I guess the problem is that in here total means area integral whereas in landIceFreshwaterFluxTotal total means frazil+interface

@xylar
Copy link
Collaborator Author

xylar commented Feb 28, 2024

guess the problem is that in here total means area integral whereas in landIceFreshwaterFluxTotal total means frazil+interface

Yes, I realized there's that conflict in meaning. We've been using "total flux" for a while to mean the integrated flux. But we could change it to integrated instead for clarity.

Also plot constituents: the melt rate at the interface and the
frazil flux below ice shelves.
We still use the old variable if necessary for backwards
compatibility.
@xylar xylar force-pushed the switch-to-land-ice-freshwater-flux-total branch from 6bf8b0d to 20dd7ca Compare February 28, 2024 19:15
@irenavankova
Copy link
Contributor

This looks good to me.

@cbegeman
Copy link
Collaborator

I agree. Thanks for making those changes!

@xylar xylar merged commit b01ad0a into MPAS-Dev:develop Feb 29, 2024
5 checks passed
@xylar xylar deleted the switch-to-land-ice-freshwater-flux-total branch February 29, 2024 08:02
@xylar
Copy link
Collaborator Author

xylar commented Feb 29, 2024

Thanks so much @cbegeman and @irenavankova!

xylar added a commit to xylar/MPAS-Analysis that referenced this pull request Feb 29, 2024
@irenavankova
Copy link
Contributor

I have one comment here - the title on frazil plots is a bit too long. The result is that the width of the figure changes based on the season. What that means is that it is not as easy to visually compare seasonal changes by flicker the figures. Just a detail though.

@xylar
Copy link
Collaborator Author

xylar commented Mar 3, 2024

Okay, I can make another PR to fix that.

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

Successfully merging this pull request may close these issues.

3 participants