E3SM-Project / ACME-ECP

E3SM MMF for DoE ECP project
Other
9 stars 1 forks source link

Re-organize CRM physics into separate physics package #121

Closed brhillman closed 4 years ago

brhillman commented 4 years ago

Re-organize the CRM-specific code modifications needed to run MMF into a separate physics "package". This moves the bulk of the files that needed to be modified for MMF into a separate source code directory that is only built when running with use_SPCAM. This is primarily to prepare for a merge of the ACME-ECP repo back upstream to the main E3SM repo (to benefit from testing, infrastructure, and eventually potential use as a widespread physics option), but also should make for easier merges from E3SM (less conflicts in files modified both upstream and downstream), and allows removing a fair number of preprocessor directives. BFB.

rljacob commented 4 years ago

So which directories are included when use_SPCAM is true?

brhillman commented 4 years ago

@rljacob building with use_SPCAM will include everything in components/cam/src/physics/crm, and whichever subdirectories are needed for the specific configuration. The code in components/cam/src/physics/cam still is included, because we use much of the CAM physics both in the background for diagnostics and also to use the utilities provided therein.

rljacob commented 4 years ago

That's confusing. What about the files that have the same name between physics/crm and physics/cam ?

brhillman commented 4 years ago

@rljacob I agree it's confusing, but which part do you object to specifically? There's two issues here that I can see. First, the fact that CRM and non-CRM physics run side by side. To clarify just a bit, this is done both to get diagnostic information about what the non-MMF physics would produce for the given model state each timestep, but also more importantly for the reason that we actually rely on the non-CRM physics in some places. In particular I think we need some things from the deep convection for aerosol processes, but I'm not too familiar with this. We've wanted to separate this for a long time, but every time we look at it, it becomes apparent that it would require a significant amount of work, if even possible.

The second issue that I see is how the build works, and having duplicate-named files in different locations. To answer your question, where there's duplicates between physics/crm and physics/cam, the path that is added in the configure script first is the version that gets built. This is how the radiation used to be built, with a default interface in physics/cam and then a different interface in physics/rrtmg which would override the version in physics/cam via configure. I ended up changing this in the radiation build, but this was really only possible because the different radiation packages could be completely separated, and I could build either all of camrt, or all of rrtmg with no overlap. To do this with separate packages for the CRM and CAM physics, we would need to duplicate EVERY file in the physics/cam directory over to physics/crm, most of which we would not need to modify at all. That seems a bit more confusing, but I'm open to alternatives here. The other option, of course, is to litter the existing physics/cam directory with ifdefs, but that's my least favorite approach.

rljacob commented 4 years ago

Its the second issue that I'm concerned about. Can you add a README to physics/crm explaining how that works? For future developers who may not know.

whannah1 commented 4 years ago

RE: needing CAM physics - I've gotten confused about this several times, but my current understanding is that we DO NOT actually need most of it. I haven't dug into the changes in this PR yet, but I would be in favor of removing all the "offline" physics parameterizations in the version of physpkg.F90 that will be used for the MMF.

One thing that makes me confident about this is the way Cheryl Craig reorganized the code for SP-CAM a few years ago. She made a new file that only contained a new "tphysbc" and removed all the convective parameterization calls. So this confirms that they are not needed for aerosol transport or CMT. We may want to make sure no one tries to run the 1-mom MMF with prognostic aerosols, because I think this actually did work before, but it's not a viable configuration.

I actually have found the diagnostic output useful when I was implementing ESMT, but I could have addressed this other ways, so I don't think we should worry about losing the diagnostic output and just trash it.

brhillman commented 4 years ago

@rljacob yeah that's no problem. I agree it's confusing, and I'd like to fix the way we do this, but I think that will take some more work and mostly I'm just trying to reduce the headache we are going to have getting this merged back upstream into E3SM. So hopefully this will be a temporary fix. I'll push a commit with a README though to clarify. It would be great to put some general information about the CRM physics package in there anyways. Great idea.

@whannah1 I'd love to be able to drop the requirement of running the "offline" CAM physics, so we should definitely explore that. Maybe in a separate PR? We could maybe still maintain an ability to run a second physics package in a diagnostic (no tendencies updated) mode at runtime maybe, if we were smart about how we did it.

whannah1 commented 4 years ago

@brhillman the advantage of removing the offline stuff here is that it would make the PR smaller. I think the idea of running the other physics diagnostically could work if we named things differently for the MMF phys package, otherwise we'd have to deal with building two files with the same name.

brhillman commented 4 years ago

@whannah1 okay, I can revisit this on this PR then.

whannah1 commented 4 years ago

A comment for clarification: It turns out that disabling the offline physics is more difficult than we anticipated. I've documented what we learned from digging in this issue on this confluence page: https://confluence.exascaleproject.org/display/ADSE15/Offline+Physics We definitely need to punt the offline physics issue to another PR and have this one focus on separating the physics packages to help the merge into E3SM.

whannah1 commented 4 years ago

Closing this since the work was all redone on the E3SM repo.