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

Dephy forcings #34

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Dephy forcings #34

wants to merge 13 commits into from

Conversation

sjboeing
Copy link
Contributor

@sjboeing sjboeing commented Feb 3, 2021

@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:

  • I have also worked on makefile compilation here. This works on my laptop now, but compiling with SOCRATES is temporarily disabled.
  • One commit addresses a bug in the interpolation scheme, which is of wider use.
  • The DEPHY routine replaces the profile initialisation done by the grid manager, as well as setfluxlook, coriolis, and the "forcing" (from mcf) routines. I have implemented some sanity checks to check for inconsistent initialisation.
  • The routine should be run when MONC is in "entire domain" mode, not "column mode". I am working on a sanity check for this.

@sjboeing
Copy link
Contributor Author

sjboeing commented Feb 3, 2021

PS: apologies for all the whitespace changes.

Steven Boeing added 4 commits February 3, 2021 14:19
Conflicts:
	components/conditional_diagnostics_whole/src/conditional_diagnostics_whole.F90
	components/pdf_analysis/src/pdf_analysis.F90
Copy link
Collaborator

@leifdenby leifdenby left a 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
Copy link
Collaborator

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?

Copy link
Contributor Author

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
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is RMDI? :)

Copy link
Contributor Author

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)

@sjboeing
Copy link
Contributor Author

sjboeing commented Feb 3, 2021

@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.

@sjboeing
Copy link
Contributor Author

sjboeing commented Feb 4, 2021

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
Copy link
Collaborator

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"?

Copy link
Contributor Author

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
Copy link
Collaborator

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=&
Copy link
Collaborator

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?

Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants