-
Notifications
You must be signed in to change notification settings - Fork 147
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
~~Add NRL_parse_tracers.F90 as a replacement for GFDL_parse_tracers.F90 for NEPTUNE~~ Remove GFDL_parse_tracers.F90 #1087
base: main
Are you sure you want to change the base?
Conversation
@climbfuji I can't recall why I put the GFS_parse_tracers file in the MP/GFDL subdirectory. Most likely because of the name :) But yeah, it doesn't make sense to have it there. Actually, I don't this code belongs in the physics at all, and should probably live with the host (in its typedefs). For the UFS, we could put the get_tracer_index() function here, and somewhere analogous for NEPTUNE? |
Do you recall why it's been added to ccpp-physics at all? |
I have no insight on its history. |
@dustinswales @climbfuji I don't understand why this is in the physics at all. It should be part of the host model. This goes for both GFDL_parse_tracers and NRL_parse_tracers. The fact that it was seen fit to make an NRL-specific version tells us that this should probably be in the host too. The |
@climbfuji @grantfirl |
Ok. NEPTUNE is trying to follow the UFS directory structure for CCPP. How about we move the file into FV3/ccpp/data in the UFS, and similar for NEPTUNE? |
I'll convert this back to a draft PR but keep it open until we've made the changes we discussed. |
A PR was created in the NRL enterprise github to remove both files, |
…ers.F90; update CODEOWNERS physics/docs/ccpp_doxyfile
I've opened this PR up for reviews again. I did not create the necessary PR in fv3atm for it, and it's not entirely clear to me how we will do this - this PR is for NCAR ccpp-physics main, not for the ufs/dev branch in the ufs-community fork. Please advise. |
@climbfuji Thanks for modifying the PR to move this file to the host's control. |
@climbfuji I created a PR into the SCM with this change. See NCAR/ccpp-scm#520 |
@dustinswales @grantfirl We merged the corresponding PR in the NRL fork that removes both |
This PR has been updated based on the discussion below. It now removes GFDL_parse_tracers.F90 and all mentions of it.
Original PR description
This PR cherry-picks an update from the NRL fork that adds
NRL_parse_tracers.F90
as an alternative forGFDL_parse_tracers.F90
for NEPTUNE. It translates the tracer names used inGFS_typedefs
(stemming from the FV3 dycore at GFDL) into the correct names used at NRL, otherwise it is the same as the GFDL version.The motivation for doing it this way is so that the code in NRL's
GFS_typedefs
can stay the same as in the UFS to facilitate future updates back and forth. The originalGFDL_parse_tracers.F90
hasn't change since the beginning of time, and therefore I do not anticipate changes to the NRL version unless we start using a new tracer in NEPTUNE with a different name than the FV3 name.Note. Because the NRL fork is behind NCAR main, this PR currently adds the file in the original location of
GFDL_parse_tracers.F90
- inphysics
. I was hoping to move it to the new location ofGFDL_parse_tracers.F90
, but then I realized this file is now inphysics/MP/GFDL
- probably not the best place. This file has nothing to do with GFDL microphysics. It's a "hook" or a "tool" that is used to parse tracer names from the FV3 dycore (and by extent the UFS) in an array, entirely independent of the choice of microphysics. Therefore, I suggest that I move the NRL version to eitherhooks
ortools
in this PR (whatever @dustinswales and @grantfirl think is best) and then create an issue that someone movesGFDL_parse_tracers.F90
to the same location (this will require metadata changes, for example inGFS_typedefs.meta
, where it is specified as a dependency).