-
Notifications
You must be signed in to change notification settings - Fork 18
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
Dephy forcings #34
base: master
Are you sure you want to change the base?
Dephy forcings #34
Conversation
PS: apologies for all the whitespace changes. |
Conflicts: components/conditional_diagnostics_whole/src/conditional_diagnostics_whole.F90 components/pdf_analysis/src/pdf_analysis.F90
…ation without CASIM
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.
Yes, it's a little hard to work out which files have actually change with all the whitespace changes :) But they weren't supposed to be there anyway.
A quick look over the files and to me it looks the files that have change that might affect functionality are:
- components/componentheaders.static
- components/componentregistrations.static
- components/conditional_diagnostics_whole/makefile
- components/conditional_diagnostics_whole/src/conditional_diagnostics_whole.F90
- components/dephy_forcings/makefile
- components/dephy_forcings/src/dephy_forcings.F90
- components/gridmanager/src/gridmanager.F90
- components/iobridge/makefile
- components/lowerbc/src/lowerbc.F90
- components/makefile
- components/profile_diagnostics/makefile
- components/setfluxlook/src/setfluxlook.F90
- components/subgrid_profile_diagnostics/makefile
- components/tvdadvection/makefile
- dephy_check.mcf
- global_config
- makefile
- model_core/src/configuration/checkpointnetcdfparser.F90
- model_core/src/grid/interpolation.F90
And not:
- components/pdf_analysis/src/pdf_analysis.F90
Does that sound right? How do you feel about splitting the changes into three separate pull requests? I was thinking 1) changes to makefiles, 2) bugfix for interpolation and 3) dephy forcings. I can help with that and we can merge in 1) and 2) immediately in that case.
@@ -22,6 +22,7 @@ use meanprofiles_mod, only : meanprofiles_get_descriptor | |||
use modelsynopsis_mod, only : modelsynopsis_get_descriptor | |||
use petsc_solver_mod, only : petsc_solver_get_descriptor | |||
use pressuresource_mod, only : pressuresource_get_descriptor | |||
use tvdadvection_mod, only : tvdadvection_get_descriptor |
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.
any idea why this moved?
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 can check if this is strcitly needed. I had some trouble getting the order of compilations for makefiles right, but it may just be a matter of getting the order correct in the makefile.
public conditional_diagnostics_whole_get_descriptor | ||
Real, Parameter :: RMDI = -32768.0*32768.0 |
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.
what is RMDI
? :)
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 is a missing data indicator (see e.g. http://cms.ncas.ac.uk/documents/IDL/idl_guide.html#missing)
@leifdenby: With gitk it is possible to filter out the whitespace, but I haven't been able to do this on github so far. I think number 2 (the bugfix for interpolation), will be quick to implement as a separate bugfix and I can do this tomorrow. Number 1 (the makefile compilation) could be a separate pull request as well. However, it breaks existing functionality as the moment, so I would not pull this into the master branch right now. On the other hand, I am currently using it to compile MONC, though I think fcm works as well. Apologies for the somewhat sloppy pull request, the main thing will be the new dephy_forcings routine though. |
Another issue I have been thinking about is how to read in forcings that are in the "normal DEPHY" format, rather than our "IDEPHYX" format (Implémentation de DEPHY avec des eXtensions ). The difference between the formats is that IDEPHYX has some additional time-dependent fields, which are characterised by their name having the subscript "_traj". There is also a quirk in DEPHY that I currently haven't dealt with, which are inputs which can be either a string or an integer (I have simply used integers for these). I think eventually, we would want to check whether the "_traj" variable is present, and otherwise use the non-"_traj" ones in fortran. Throw a wobbly when none or both are present. This will make the code a bit ugly. As a first step, I could write a script that converts DEPHY to IDEPHYX. |
if (is_component_enabled(current_state%options_database, "forcing")) then | ||
call log_master_log(LOG_ERROR, "DEPHY: forcing component incompatible with dephy forcing") | ||
endif | ||
if(is_component_enabled(current_state%options_database, "socrated_couple")) then |
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.
just checking - is this meant to be "socrated" and not "socrates"?
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.
You are right (also below), this should be socrates.
call log_master_log(LOG_ERROR, "DEPHY: socrates_couple component incompatible with dephy flag rad_theta==1") | ||
endif | ||
endif | ||
if(.not. is_component_enabled(current_state%options_database, "socrated_couple")) then |
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.
just checking - is this meant to be "socrated" and not "socrates"?
call dephy_set_flux(current_state) | ||
|
||
current_state%fbuoy=0. | ||
current_state%fbuoy=& |
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.
why is this set to zero then redefined?
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.
Yes, this can be removed. It is a remnant from copying it from setfluxlook.F90, where there is an option to run with temperature or moisture fluxes only.
@leifdenby @cemac-ccs : Here is some preliminary work on the DEPHY forcing implementation. It will need further improvements, cleaning, testing and documentation before it can go in.
Notes: