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

Adds a 1D forcing testcase with BGC (ISPOL) #5282

Closed
wants to merge 3 commits into from

Conversation

njeffery
Copy link
Contributor

@njeffery njeffery commented Nov 9, 2022

Atmospheric, ocean and biogeochemical data from the 2004 Ice Station Polarstern (ISPOL) Weddell Sea (67.9S, 54W) experiment from June 16, 2004 to December 31, 2004. Core 2 from the ISPOL location fills out the remainder of the year.

A uniform_1D initial condition was added for flexible initialization. A new testcase - single_cell_ispol - was added to
MPAS-Dev/MPAS-Seaice_standalone_framework to verify this forcing and initializtion. MPAS-Dev/MPAS-Seaice_standalone_framework#15

BFB

Atmospheric, ocean and biogeochemical data from the 2004 Ice Station Polarstern (ISPOL)
Weddell Sea (67.9S, 54W) experiment from June 16, 2004 to December 31, 2004.
Core 2 from the ISPOL location fills out the remainder of the year.

A uniform_1D initial condition was added for flexible initialization.
A new testcase - single_cell_ispol - was added to
MPAS-Dev/MPAS-Seaice_standalone_framework to verify this forcing and initializtion.

BFB
@rljacob
Copy link
Member

rljacob commented Dec 1, 2022

@akturner and @eclare108213 please review.

@eclare108213
Copy link
Contributor

Darin and I spent several hours yesterday trying to get me up and running on anvil, and failed. Adrian's testing setup and even E3SM itself fail to run (E3SM segfaults). So please don't hold up merging PRs, waiting on reviews from me. I'll keep trying, but E3SM has been failing for me ever since it was called ACME.

units="W m-2"
description="Incoming longwave flux (positive down)"
/>
<var name="airTemperature1D"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this implementation could be greatly simplified if the input forcing files included a nCells=1 dimension so that an airTemperature1D variable etc in addition to the airTemperature variable would not be needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@akturner : Agreed. I chose to make the forcing data grid independent though so I could test on an arbitrary grid. It runs with the QU240, for example, at least in short tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

@njeffery If the forcing is the same everywhere on the mesh, what's the utility of being able to run on meshes other than the single cell one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@akturner : your point is well taken. I'll restructure the test case with a cell index in the forcing files. I'd like to keep the bgc nutrient file grid independent so that I can use it more generally, i.e. in a standard GCASE but with ocean bgc off. Does that seem reasonable?

@@ -558,6 +564,124 @@ subroutine init_ice_state_uniform_ice(&

end subroutine init_ice_state_uniform_ice!}}}

!|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
!
! init_ice_state_uniform_1D
Copy link
Contributor

Choose a reason for hiding this comment

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

Do thermodynamic tracer values not need to be set by this testcase with colpkg_init_trcr (e.g. https://github.com/E3SM-Project/E3SM/blob/master/components/mpas-seaice/src/shared/mpas_seaice_initialize.F#L705?) It seems from the IPSOL namelist file that vertical thermodynamics is active so would need the tracers initialized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@akturner : Yes, thanks. I added the active tracer initializations and re-tested.

Properly initializes enthalpy and salinity using colpkg_init_trcr
@rljacob
Copy link
Member

rljacob commented Jan 5, 2023

@eclare108213 please review.

@eclare108213
Copy link
Contributor

The conflict needs to be fixed before I can compile this. @njeffery what cases did you run that were BFB? Is there a particular test (or test suite) that would be most appropriate for me to run, to make sure that something hasn't inadvertently been broken in E3SM?

@njeffery
Copy link
Contributor Author

njeffery commented Jan 6, 2023

@eclare108213 : I ran a short 10-day with restart GCASE on chrysalis with mpassi defaults and compared mpassi restarts with master. These were BFB. Simulation scripts for these runs are here /home/ac.jeffery/SimulationScripts/archive/v2/ with run.ISPOLTestGCASE.sh using the feature branch and run.ISPOLTestGCASE.Master.sh pre-feature.

@rljacob
Copy link
Member

rljacob commented Feb 16, 2023

@akturner have your comments been addressed?

@njeffery
Copy link
Contributor Author

@rljacob : I still have some changes to make in response to @akturner's comments.

@rljacob
Copy link
Member

rljacob commented Mar 2, 2023

@njeffery how are those changes going?

@njeffery
Copy link
Contributor Author

njeffery commented Mar 6, 2023

I've opened a new PR ( #5506 ) to address the issues raised by @akturner . This one can be closed.

@jonbob jonbob closed this Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB mpas-seaice
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants