-
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
CICE: Floe size distribution #382
Conversation
…f not tested; no pio
And the build fails because it has the older Icepack version, which will be fixed when the new PR is merged and updated in CICE. |
Right, reverting back to the current Icepack version in CICE is not what we wanted. When we committed the ifsd1 PR to the Icepack, I pushed an update to @lettie-roach fsd branch to point to that. We'll need to do that again once we push ifsdech branch to the Consortium. The fact it's failing now is OK, I'll update Icepack again when we push the next PR. |
Okay, I'll try to stay out of the way! |
Except for moving the ww3 file to ftp and final testing, this PR looks ready to merge. |
I think this is ready to merge. The testing looks good. https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks hash a7080a4. I will merge this now. |
Hi Everyone,
In regard to the FSD, I am running into an issue where in ice_in, if I specify
tr_fsd = .false.,
nfsd = 0
The run stopps with error trying to define history file ‘nf’ as an unlimited dimension:
(abort_ice)ABORTED:
(abort_ice) error = (ice_write_hist)ERROR: defining dim nf
(abort_ice)NetCDF: NC_UNLIMITED size already in use
The issue is in ice_history_write.F90: line 193, where fsd is defined without a check if tr_fsd is defined.
Question: Should we put a check if tr_fsd = .true. before defining ‘nf’ as a dimension? OR is this handled differently in other tracers? I can make that change and submit a pull request if needed.
Thanks for the help and advice!
David
David A. Hebert
Ocean Sciences Division, Code 7322
Naval Research Laboratory
Stennis Space Center, MS 39529
(228)688-5846; DSN 828-5846
http://www.nrl.navy.mil <http://www.nrl.navy.mil/>
From: Tony Craig <[email protected]>
Sent: Saturday, December 07, 2019 02:16
To: CICE-Consortium/CICE <[email protected]>
Cc: Subscribed <[email protected]>
Subject: Re: [CICE-Consortium/CICE] CICE: Floe size distribution (#382)
Merged #382 <#382> into master.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub <#382?email_source=notifications&email_token=AE52VPBWN2HPVTHH5PXUQJDQXNLT3A5CNFSM4JSMZ23KYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOVKVV6GY#event-2863357723> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE52VPBQIWCXRVPDHWNNENLQXNLT3ANCNFSM4JSMZ23A> . <https://github.com/notifications/beacon/AE52VPHICT6YX5WAHIJJFSDQXNLT3A5CNFSM4JSMZ23KYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOVKVV6GY.gif>
|
If you are running with any ice at all, you ought to have at least one floe size category! I would think that nf=1 should be the default when tr_fsd=F, but maybe it's not. |
Maybe this is a documentation item. We should put in there that nf must be .ge. 1. @duvivier - do this asap! |
Physically, that makes sense.
Code wise, when tr_fsd = .false., is there any output that will have ‘nf’ as a dimension? I suppose it doesn’t hurt to have a dimension that isn’t used in the history NetCDF. Just trying to avoid extraneous definitions and see what is consistent with the other tracers when they are false.
Thank you,
David
David A. Hebert
Ocean Sciences Division, Code 7322
Naval Research Laboratory
Stennis Space Center, MS 39529
(228)688-5846; DSN 828-5846
<http://www.nrl.navy.mil/> http://www.nrl.navy.mil
From: Elizabeth Hunke <[email protected]>
Sent: Monday, January 06, 2020 15:27
To: CICE-Consortium/CICE <[email protected]>
Cc: David Hebert, Code 7322 <[email protected]>; Comment <[email protected]>
Subject: Re: [CICE-Consortium/CICE] CICE: Floe size distribution (#382)
If you are running with any ice at all, you ought to have at least one floe size category! I would think that nf=1 should be the default when tr_fsd=F, but maybe it's not.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#382?email_source=notifications&email_token=AE52VPEUAAN66PY6I3CO5UDQ4OO4BA5CNFSM4JSMZ23KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIG246I#issuecomment-571321977> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE52VPH7OWKMVOYXJQWDLB3Q4OO4BANCNFSM4JSMZ23A> . <https://github.com/notifications/beacon/AE52VPC4HGEWLIHPQWIWF43Q4OO4BA5CNFSM4JSMZ23KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIG246I.gif>
|
Agreed. If the FSD is off, then there is no need to have nf in the history output. However it doesn't take up much space and would make it clear exactly how many floe size categories were in use. Also, if someone's analysis software expects it to be there and breaks because it isn't, they might be a little annoyed (but that just shows lack of flexibility in their software). I think we should make it clear in the doc that nf >= 1, as @duvivier suggests. I'm open to others' opinions about whether nf should be included in the output when it isn't used as a netcdf dimension. |
I would argue if there is a clean way to turn off fsd (tr_fsd) then the value of nf should not matter and it should be fine to be zero or anything else when fsd is off. And that (tr_fsd) should also control whether the fsd output is added to the history/restart or not. I would argue that this is the most appropriate way to handle all optional features / output fields. Would this be a lot of work? I'm willing to have a look and help clean this up if it's not too complicated. If it's challenging to fix properly, then I am fine with the strategy that nf >=0 for all cases. We should also add a run time check to verify a valid value. It should be more than just documentation. That same strategy should be applied to any other optional tracer that also needs to be >=0 even if it's not turned on. |
It is fine with me for tr_fsd to control whether nf is added to the history/restart files. @apcraig I accept your offer to take a look and clean it up if it's straightforward! Thanks. However I do think that the default value should be nf=1 when FSD is off, regardless of whether it's written to the files, because that is the physically meaningful value. It's possible that some other parameterization will need/want to use it in the future. |
This '+' sign was copy-pasted there in error in 29b99b6 (CICE: Floe size distribution (CICE-Consortium#382), 2019-12-07). Remove it.
This '+' sign was copy-pasted there in error in 29b99b6 (CICE: Floe size distribution (CICE-Consortium#382), 2019-12-07). Remove it.
This '+' sign was copy-pasted there in error in 29b99b6 (CICE: Floe size distribution (CICE-Consortium#382), 2019-12-07). Remove it.
This '+' sign was copy-pasted there in error in 29b99b6 (CICE: Floe size distribution (CICE-Consortium#382), 2019-12-07). Remove it.
This '+' sign was copy-pasted there in error in 29b99b6 (CICE: Floe size distribution (CICE-Consortium#382), 2019-12-07). Remove it.
* drivers/hadgem3: add missing 'subname' and use existing 'subname's * drivers/hadgem3/CICE_InitMod: update 'init_lvl' call Add the required 'iblk' argument. * drivers/hadgem3/CICE_RunMod: remove uneeded 'dt' arguments The subroutines 'prep_radiation', 'zsal_diags', 'bgc_diags' and 'hbrine_diags' do not take a 'dt' argument anymore, so remove it. * drivers/hadgem3/CICE_RunMod: get 'Lsub' from Icepack * drivers/hadgem3/CICE_RunMod: remove 'da_state_update' subroutine This subroutine is inside an 'ICE_DA' CPP, which is not used in any configuration. Remove it. * drivers/hadgem3/CICE_RunMod: remove stray '+' This '+' sign was copy-pasted there in error in 29b99b6 (CICE: Floe size distribution (#382), 2019-12-07). Remove it. * drivers/hadgem3: remove obsolete 'check_finished_file' subroutine Remove the call to 'check_finished_file' as well as the definition of the subroutine, as the 'hadgem3' driver is not used on machine 'bering' and it's unclear if machine 'bering' still exists. * drivers/hadgem3: fix cice_init so it calls 'count_tracers' This was forgotten back in 8b0ae03 (Refactor tracer initialization (#235), 2018-11-16) * drivers/hadgem3/CICE_RunMod: add call to 'save_init' The hadgem3 driver was not updated when 'save_init' was added in 83686a3 (Implement box model test from 2001 JCP paper (#151), 2018-10-22). As this subroutine is necessary to ensure proper initialization of the model, add it now. * drivers/hadgem3/CICE_RunMod: tweak loop indices in 'coupling_prep' Other drivers use 'ilo,ihi' and 'jlo,jhi' here. Do the same. * drivers/hagdem3: update driver to new time manager * drivers/hadgem3: pass 'mpi_comm_opa' explicitely to init_communicate In 066070e (Fix minor issues in documentation, key_ CPPs, bfbcomp return codes (#532), 2020-11-23), the 'ice_communicate' module was updated to remove CPP macros relating to the OASIS coupler (key_oasis*) and to the NEMO ocean model (key_iomput). These CPPs were used to make the correct MPI communicator accessible to the 'init_communicate' subroutine. However, that subroutine already accepts an optional MPI communicator as argument and it was deemed cleaner to require the driver layer to explicitely pass the communicator instead of making it accessible through 'use' statements. Update the 'hadgem3' driver, used for coupling with NEMO, to explicitely pass the NEMO MPI communicator 'mpi_comm_opa' to 'init_communicate'. * drivers: add 'nemo_concepts' driver Historically the 'hadgem3' driver has been used when compiling a single NEMO-CICE executable at ECCC. Going forward, all driver-level adjustements will be done in our own driver, 'nemo_concepts', 'CONCEPTS' being the name of the multi-departmental collaboration around using the NEMO ocean model. Copy CICE_InitMod, CICE_RunMod and CICE_FinalMod from the 'hadgem3' directory to a new 'nemo_concepts' directory under 'drivers/direct'. The following commits will clean up this new driver and port over some in-house adjustments. * drivers/nemo_concepts: remove unused 'writeout_finished_file' subroutine This subroutine was only called on machine 'bering', which is not an ECCC machine and probably does not exist anymore anyway. Remove it. * drivers/nemo_concepts: call 'scale_fluxes' with 'aice_init' Since 'merge_fluxes' is called with aice_init, it is more consistent to also call 'scale_fluxes', in 'coupling_prep' with 'aice_init' instead of 'aice'. Copy this in-house change to the new 'nemo_concepts' driver.
PR checklist
Changes to CICE driver to implement floe size distribution in Icepack.
Lettie Roach, Elizabeth Hunke
Passes basic tests; currently undergoing more extensive testing
This PR is not yet ready to merge, pending
See also FSD project board