-
Notifications
You must be signed in to change notification settings - Fork 132
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 reading of mixed layer forcing file. #578
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks OK to me. Recommend additional reviews for model setup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In ice_forcing.F90, the currents might still be too strong, but you can try it like this.
Instead of moving init_forcing_ocn later in the subroutine, would it work to move init_forcing_atmo up? sst is needed in init_state.
Doesn't ocn_data_dir need to be generic in set_nml.ml, as for other (atmo) forcing?
Very good points. I'm actually now thinking I could move the call to allocate the arrays to init_forcing_ocn instead and leave the calls in the current location in CICE_InitMod.F90. You are right about ocn_data_dir as well. |
Sorry for the repeated commits/pushes. I had troubles with conflicts from the new time manager stuff. |
Looks like I still have some issues. |
@dabail10 since you've gotten some runs completed, have the issues mentioned above been fixed? |
I think the code is fixed, but we don't have the forcing file on Zenodo yet. |
I'm confused, why was this closed and not merged? |
Not sure. I think it needs the forcing file? |
@dabail10, it looks like you closed this. we can reopen it. I thought this had to be merged regardless of whether there is an available file on zenodo. |
I can reopen. Not sure what happened. |
I see what happened. I thought it was merged and I deleted the branch. My bad. |
ocn_data_type = 'ncar' | ||
ocn_data_format = 'nc' | ||
ocn_data_dir = 'ICE_MACHINE_INPUTDATA/CICE_data/forcing/gx1' | ||
oceanmixed_file = 'pop_frc.gx1.nc' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need have a standard name for the oceanmixed_file and then commit to Zenodo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about ocean_forcing_gx1.nc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be named similarly to the atmospheric forcing, but that is not standardized. It could follow the JRA tx1 example,
CESM_ocean_monthly_forcing_gx1.nc -- probably overkill since its location in the forcing directory structure would identify it: CICE_data/forcing/gx1/CESM/MONTHLY/
So ocean_forcing_gx1.nc works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eclare108213 mentioned having 2D in the name as well. Then if we put it under the forcing/gx1 area, we shouldn't need the "gx1" label.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want a filename that doesn't rely on it's directory location to describe it. We also don't want the same filename in multiple directories. So we want to avoid having multiple "ocean_forcing.nc" files in multiple directories. When the file is separated from it's directory, it should be unique (relative to other input data files) and still somewhat descriptive. I am completely fine with something like "CESM_ocean_monthly_climatology_forcing_gx1_210324.nc". But it need not be that descriptive. It's always a balance. When/if we update this file in the future, we'll need to give it a new name as well. We need to make sure we never have multiple versions of the same filename, ever.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added the Kay et al. 2015 reference to the CESM-LE in the metadata. I think the metadata can be very descriptive as well as the Zenodo entry. I believe we should keep the filename simple.
I would go with more descriptive rather than less.
|
Yes, the metadata will have essential info, so we don’t need that kind of distinguishing label on the file name. I still prefer including gx1, and 2D is helpful. So maybe ocean_forcing_clim_2D_gx1.nc?
|
I like that. I will fix the PR. |
Updated set_nml.ml option. The forcing file has been added to Zenodo at 10.5281/zenodo.4633898. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this is ready to merge. @eclare108213 ?
The new forcing file and location needs to be added to the input data wiki page. Otherwise this is ready. Thank you! |
The forcing file information was added to the input data page. |
For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers
PR checklist
This fixes a bug where the mixed-layer forcing was not initialized correctly as the array was not allocated.
dabail10 (D. Bailey)
https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_mach_forks#cheyenne
I have added a new test, set_nml.ml that turns on oceanmixed_ice with a SOM forcing file from a CESM run. The main change is to move the call to init_forcing_ocn in CICE_InitMod.F90. Also, the default has changed to use a 2D forcing file with everything at the gridcell centers. I've also added some helpful extra diagnostic information in the netCDF read routines.