-
Notifications
You must be signed in to change notification settings - Fork 25
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
Bugfix: write tracer restart files for all experiments. #124
Bugfix: write tracer restart files for all experiments. #124
Conversation
Not sure whether we will need restarts for other types of configurations than 'cesm' in the future, but for now this should do the job. Thanks Tomas |
Although I have not tested it in a while, tracer restart should work for other tracers than HAMOCC's for all experiment configurations. This is done in restart_ocntrcwt, called by restart_trcwt, and it will not be possible to write restart for these tracers if the test on expcnf is done as suggested in this PR. Would a possibility be to place the expcnf test inside the HAMOCC CPP block of restart_trcwt? Of course it would be good to be able to restart HAMOCC in some standalone configurations, even if just for efficient testing. |
I got confused by some preprocessing flag settings. I think I will try to replicate the |
a6c0fe1
to
398e5f4
Compare
@matsbn @JorgSchwinger The new version of the tracer restart setup defines restart file names in Also, this removes the option to read in HAMOCC restart files from micom runs, but it looks like this option has already been removed elsewhere in the code. Is this OK, or should we aim to keep this functionality in place in master? Finally, should this bug fix be applied also for the release-1.0 branch? If so, I should probably rebase the bugfix branch to a common master / release-1.0 ancestor. |
Hi Tomas, I didn't have a closer look, but we definitely don't want to remove the capability to restart from restart files with '.micom.' in them. We don't need to write such files, but we want to restart from them. |
I agree with @JorgSchwinger that we should still be able to restart from '.micom.' files. |
@matsbn @JorgSchwinger I tried to run a hybrid run with restart files from |
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.
From the HAMOCC side, this looks ok, although I suspect that the error you get somehow is related to these changes.
This should be thoroughly tested in all possible combinations (with/without PNETCDF) and with .blom. and .micom. restart files, and also in coupled configuration.
I agree that some more testing is needed. I am able to run N1850 with I was able to trace the error to 1 day difference between the BLOM internal clock and the clock for the coupled system. In |
Create filenames in restart_trcrd/restart_trcwt. Pass filenames as arguments to ocntrc and hamocc rd/wt subroutines.
d2d4f62
to
afd854c
Compare
With respect to the clock difference, it seems the internal BLOM date has not been correctly incremented to take into account that with hybrid run type, the ocean component will start after the first coupling time step. What is the ocean coupling frequency in the test you did, @TomasTorsvik? |
@matsbn |
With OCN_NCPL=1 it is daily ocean coupling. Probably, the time management change associated with the major code restructuring (commit e2a4230) has not been properly tested with daily coupling (most CMIP6 simulations uses sub-diurnal coupling frequency). This should of course be corrected and is likely unrelated to the restart filename generation of this PR. |
Thanks for the update. Should we make a new issue for the ocean coupling frequency? |
Also, I am getting errors when running with The |
This is probably the T62_tn21 resolution? I've been running this a lot, and I think it also worked after the major code restructuring. I'll test this tomorrow. |
Yes, it is T62_tn21. I have been using restart files from with |
I included the PnetCDF module for the test run on Betzy. I get bit identical output for N1850_piControl run when comparing with noresm2.0.5 (without pnetcdf), for daily output after 1 year, so I don't think there are any issues with |
I did a short test with the NorESM release 2.0.5 (but with the current version of blom master), and I cannot reproduce any restart issue with the T62_tn21 resolution and daily coupling frequency. I restarted from NOINYOC_T62_tn21_betzy, and also from restart files generated during the run and didn't have any problems. This suggests it must have something to do with the changes related to this PR. |
@JorgSchwinger
|
Ok, I have never tested a hybrid restart for this. I just tested restarting. |
Normal restarting ("RESUBMIT", "CONTINUE_RUN") has been tested and works fine, so at least it doesn't break anything that was already working. I suppose hybrid restart is a separate issue (I already created one in the issue tracker). |
Suggestion for bugfix. This prevents call to
restart_trcwt
ifexpcnf
is not'cesm'
.It would probably be better to make the tracer restart file generation more general, but this may require some restructuring of the restart writing procedure. At the moment there are dependencies between
restart_wt
,restart_ocntrcwt
andaufw_bgc
in terms of the restart filename.