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

Add freshwater tracers #108

Conversation

cbegeman
Copy link
Collaborator

This PR adds a freshwater tracer group that tracks the concentration of all surface mass fluxes to the ocean. The nonlocal flux term is not included because it would require a new nonLocalSurfaceTracerFlux struct array to separate the salinity flux into its constituent mass source terms. That term is also not included for other passive tracers.

@cbegeman
Copy link
Collaborator Author

@jonbob When you have time, would you be willing to take a look at this PR and let me know if you see any obvious issues why E3SM builds would fail while MPAS-O builds are successful? I imagine I might have missed something that your namelist-related scripts would pick up.

Here's the build log if that's helpful. I didn't see anything that looked informative to me. /lcrc/group/e3sm/ac.cbegeman/scratch/chrys/SMS_Lm1_P2048.ne30pg2_ECwISC30to60E2r1.CRYO1850-DISMF.chrysalis_intel.G.20240702_105706_xqyt5o/bld/e3sm.bldlog.240702-110049

Thank you so much!

@cbegeman
Copy link
Collaborator Author

@irenavankova Here's the branch in case you'd like to start taking a look at it.

@cbegeman cbegeman added the enhancement New feature or request label Jul 11, 2024
@darincomeau
Copy link
Collaborator

darincomeau commented Jul 12, 2024

@cbegeman I see this in the bld log:

/lcrc/group/e3sm/ac.cbegeman/scratch/chrys/SMS_Lm1_P2048.ne30pg2_ECwISC30to60E2r1.CRYO1850-DISMF.chrysalis_intel.G.20240702_105706_xqyt5o/bld/cmake-bld/core_ocean/shared/mpas_ocn_subgrid.f90(550): 
error #6404: This name does not have a type, and must have an explicit type.   [CONFIG_USE_SUBGRID_WETTING_DRYING]

I don't see that config option anywhere in this PR, are you perhaps running this on top of a different branch?

@jonbob
Copy link
Collaborator

jonbob commented Jul 12, 2024

good catch, @darincomeau. Let me see if running the scripts to make bld files helps

@jonbob
Copy link
Collaborator

jonbob commented Jul 12, 2024

@cbegeman -- the problem is that we haven't brought the wetting_drying namelist into E3SM yet, so the

config_use_subgrid_wetting_drying

namelist setting isn't currently available. We can bring it in, but it might take some discussion

@cbegeman
Copy link
Collaborator Author

Thanks, @darincomeau and @jonbob! This PR doesn't depend on wetting and drying at all. How can I get around this? Is there a particular commit/tag I should rebase this onto?

@xylar
Copy link
Collaborator

xylar commented Jul 12, 2024

@cbegeman, it looks like that's the base case you're trying to compare this branch to (based on the .G. in the name). So this is with master, right?

Could it be that the test case you're trying to run isn't a good choice? SMS_Lm1_P2048.ne30pg2_ECwISC30to60E2r1.CRYO1850-DISMF.chrysalis_intel? I think we want to be testing with E3SM v3 grids. Even if so, we probably want to know why this test case is broken and fix it.

@xylar
Copy link
Collaborator

xylar commented Jul 12, 2024

I've been using:

SMS_D_P480_Ld1.ne30pg2_r05_IcoswISC30E3r5.CRYO1850-DISMF.chrysalis_intel

@xylar
Copy link
Collaborator

xylar commented Jul 12, 2024

For what it's worth, I was able to build e3sm.exe for SMS_Lm1_P2048.ne30pg2_ECwISC30to60E2r1.CRYO1850-DISMF.chrysalis_intel with the current master:

/lcrc/group/e3sm/ac.xylar/scratch/chrys/SMS_Lm1_P2048.ne30pg2_ECwISC30to60E2r1.CRYO1850-DISMF.chrysalis_intel.G.20240712_102150_3q1ix1/bld

@cbegeman, it might be worth giving it another try.

@cbegeman
Copy link
Collaborator Author

@jonbob I'm not seeing the new tracer group config options appear in the namelist for e3sm cases. When you have a chance, can you give this branch a look and see if I've missed anything in the e3sm build namelists/scripts?

@jonbob
Copy link
Collaborator

jonbob commented Jul 17, 2024

@cbegeman -- I'll look now. Oh, you have to add the new group to the groups list in build-namelist

@cbegeman cbegeman force-pushed the ocn/add-freshwater-tracers branch from 5e90fd0 to 81f93b2 Compare July 29, 2024 21:43
@irenavankova
Copy link

Past line 1819 in build-namelist add a new line tracer_forcing_freshwatertracers

@cbegeman
Copy link
Collaborator Author

Thanks for pointing this fix out @irenavankova! 81f93b2

@cbegeman cbegeman marked this pull request as ready for review July 29, 2024 21:44
@cbegeman cbegeman requested a review from xylar July 29, 2024 21:44
@irenavankova
Copy link

@cbegeman, the subglacial runoff is in master now. Do you want me to add the subglacial tracers to this pull request before moving it on?

@irenavankova
Copy link

The branch I used for the gcase runs with subglacial runoff tracers is here:

E3SM-Project/E3SM@master...irenavankova:E3SM:sgr_tracers

Probably a bit messy, so let me know if you want me to pick out something specific

@cbegeman
Copy link
Collaborator Author

@cbegeman, the subglacial runoff is in master now. Do you want me to add the subglacial tracers to this pull request before moving it on?

@irenavankova I'll rebase this branch then compare the code against your branch. I'll let you know if I have any questions or issues. Thanks!

@cbegeman cbegeman force-pushed the ocn/add-freshwater-tracers branch from 4811507 to d1febf3 Compare September 25, 2024 16:50
@cbegeman
Copy link
Collaborator Author

@irenavankova Can you rerun one of your isomip_plus configurations with subglacial fluxes with this new branch to test that capability? As far as I can tell, there's not an easy way to turn sgr on in this test case. I have rerun an isomip_plus case and it seems to be working as expected for ismf.

@irenavankova
Copy link

I ll rerun it, I have a compass branch modified for that. It might not be till tomorrow though.

@cbegeman
Copy link
Collaborator Author

@irenavankova No rush! Thanks!

@irenavankova
Copy link

@cbegeman, the isomip case with subglacial tracers runs fine on your branch.

Here is subglacial freshwater tracer in the top layer (first month average):

Screenshot 2024-10-07 at 18 00 59

And here the associated landice freshwater tracer:

Screenshot 2024-10-07 at 18 01 11

@cbegeman cbegeman force-pushed the ocn/add-freshwater-tracers branch from 8690ea8 to 119ac80 Compare February 27, 2025 21:50
@cbegeman cbegeman force-pushed the ocn/add-freshwater-tracers branch from 119ac80 to f938f02 Compare March 3, 2025 14:55
@xylar xylar changed the base branch from master to alternate March 3, 2025 15:23
@xylar xylar changed the base branch from alternate to master March 3, 2025 15:24
@cbegeman
Copy link
Collaborator Author

cbegeman commented Mar 3, 2025

@irenavankova and @xylar Would you like to see anything further before I migrate this to E3SM? I just ran the e3sm_cryo_developer's suite and all tests pass on chrys.

@irenavankova
Copy link

@cbegeman, have you tried to run with DSGR compset to make sure that the tracers work there in the current implementation? I had put the subglacial tracers in onto your branch back in July and have been running fine with that, but I am not sure if you implemented that over or used mine, the now very old branch I was running with is here: https://github.com/irenavankova/E3SM/tree/sgr_tracers

Comment on lines +1036 to +1037
subroutine ocn_surface_bulk_forcing_freshwater_tracers_subglacial_runoff(meshPool, forcingPool, &
tracersSurfaceFluxSubglacialRunoff, err)!{{{
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 Here is the routine that implements the subglacial runoff freshwater tracer if you want to take a look.

@cbegeman
Copy link
Collaborator Author

cbegeman commented Mar 3, 2025

@irenavankova If the DSGR capability is not tested in the e3sm_cryo_developer's suite, can you suggest a test to run?

@irenavankova
Copy link

I am not familiar with the standardized test suits, there was one added for DSGR though:
https://github.com/E3SM-Project/E3SM/blob/c1dca98083c6c3d88ac6f6909bdbd6caa68780d4/cime_config/tests.py#L298

If you want to I can just try to setup and run a case with DSGR manually and check that

@cbegeman
Copy link
Collaborator Author

cbegeman commented Mar 3, 2025

@irenavankova Thanks for pointing me to that. I'll run the whole e3sm_ocnice_extra_coverage suite

@cbegeman
Copy link
Collaborator Author

cbegeman commented Mar 4, 2025

@irenavankova All e3sm_ocnice_extra_coverage tests pass.

@irenavankova
Copy link

Great, thanks for checking that!

@cbegeman cbegeman closed this Mar 5, 2025
@cbegeman
Copy link
Collaborator Author

cbegeman commented Mar 5, 2025

@xylar I'm migrating this to E3SM. If you have any suggested testing or feedback you can provide it there.

@xylar
Copy link
Collaborator

xylar commented Mar 5, 2025

Sorry, yes, that's fine!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants