remove duplicate call to SoilBiogeochemVerticalProfile

[billsacks]

Bill Sacks < sacks@ucar.edu > - 2013-04-19 12:10:46 -0600 Bugzilla Id: 1668 Bugzilla CC: andre@ucar.edu, cdkoven@lbl.gov, erik@ucar.edu, muszala@ucar.edu, rfisher@ucar.edu,

Currently, decomp_vertprofiles is called from two places: (1) clm_driver.F90, (2) CNDecompAlloc, which is called from CNEcosystemDyn. From talking to Charlie, it sounds like this is a mistake; it should probably just be called from the driver.

Removing the duplicate call from CNDecompAlloc causes answer changes for transient cases, which would take some examination to confirm that this hasn't broken anything. Since we are under time pressure so close to the release freeze, we have decided not to remove this duplicate call for now. However, when someone has time to sign off on the answer changes, this should be re-examined.

[billsacks]

Bill Sacks < sacks@ucar.edu > - 2013-06-14 11:55:55 -0600

In a branch that I'm working on (where the main purpose is to make column-level filters only operate over active points), I have changed decomp_vertprofiles to operate over all istsoil & istcrop pfts & columns -- not just those that are active. I was hopeful that this would allow me to remove this duplicate call. However, when I tried removing the call from CNDecompAlloc (keeping in place the call in the driver) I get carbon balance errors in many (though not all) tests.

For example:

RUN ERS_E.f19_g16.I1850CRUCLM45CN.yellowstone_intel.GC.200453 RUN ERS_D.hcru_hcru.I_2000_CRUFRC_CLM45_CN.yellowstone_pgi.GC.200503

[billsacks]

Bill Sacks < sacks@ucar.edu > - 2013-06-14 16:03:29 -0600

I confirmed that this test also fails on the trunk (clm4_5_11):

ERS_E.f19_g16.I1850CRUCLM45CN.yellowstone_intel

if you get rid of the call to decomp_vertprofiles from CNDecompAlloc. Again, I get a C balance error a short way into the run.

[billsacks]

Bill Sacks < sacks@ucar.edu > - 2013-06-27 12:28:40 -0600

Ideally, we would also like the call to decomp_vertprofiles from the driver to be moved later in the driver sequence -- at least sometime after subgrid weights are updated due to transient land use. Currently this routine needs to operate over inactive as well as active points because of its unusually early placement in the driver sequence, and we would like to remove the need for this if possible. (The problem with its current placement is: If it just operated over active points, like most other routines, then computations wouldn't be done for PFTs that are currently 0-weight, but are about to become non-0-weight later this time step.)

[billsacks]

Bill Sacks < sacks@ucar.edu > - 2014-12-12 16:42:12 -0700

see also bug 2107

[billsacks]

Bill Sacks < sacks@ucar.edu > - 2015-02-24 12:39:37 -0700

decomp_vertprofiles has been renamed to SoilBiogeochemVerticalProfile

[billsacks]

Bill Sacks < sacks@ucar.edu > - 2016-07-15 13:04:15 -0600

I'm not sure what, if anything, should be done about this: I don't see any clearly superior solutions.

Copying an email I just sent to Charlie:

This is still called in two places:

./biogeochem/CNDriverMod.F90:318: call SoilBiogeochemVerticalProfile(bounds, num_soilc, filter_soilc, num_soilp, filter_soilp, & ./main/clm_driver.F90:210: call SoilBiogeochemVerticalProfile(bounds_clump , &

The main (only?) issue with moving this appears to be the use in dyn_cnbal_patch, which is called before CNDriver. Incidentally: While I have reworked a lot of stuff related to dynamic LU, I don't think I have changed anything related to the uses of the vertical profile stuff in dyn_cnbal_patch: their use and general timing in the driver sequencing remain as they have been over the last few years (or more).

I have given this some more (actually, too much) thought yesterday. My basic conclusion is that I can see a few possible ways forward, but don't see any easy solution. So my inclination at this point is to leave things as is, unless you or I can come up with a clear, winning solution that doesn't take too much work to put in place.

You don't need to read through all the details below, but I'm including them in case you want more details:

The options are essentially:

(1) Leave things as is, with two calls to SoilBiogeochemVerticalProfile, including a call to that and to alt_calc early in the driver sequence

Pros:

• Easy
• Safe (no risk of introducing balance errors with possible changes)

Cons:

• The current anomalous position of these in the driver sequence is a bit confusing: These routines are called much earlier than most science routines in the driver sequence
• The fact that this is called twice is also anomalous and a bit confusing
• The fact that the clm_driver calls use the "inactive_and_active" filters is also anomalous, and makes analysis of the system a bit harder
• Performance: the extra call to SoilBiogeochemVerticalProfile is responsible for ~ 1% of total CLM run time (from a very rough timing... probably not very accurate)

(1a) Keep things basically as is, but change the filters in the clm_driver call to just the active filters

I think this may work. This would eliminate or reduce some of the cons, but I'd want to do extra analysis / thought to make sure this would work.

(2) Keep the call in clm_driver, eliminate the call in CNDriver

I don't see why this would cause problems, but apparently it caused C balance errors when I tried it a while ago.

However, it might cause problems (unrelated to the previous C balance errors, I think) that there is some patch-to-column averaging in SoilBiogeochemVerticalProfile. I think that, for this solution to be robust, we'd need to separate that patch-to-column averaging into a separate routine, and do that piece later, after the subgrid area updates.

We'd still have some of the above cons – namely, the anomalous placement in the driver sequence and the operation over inactive_and_active filters. We could eliminate the latter problem (also improving performance), but then would need to re-introduce a second call that just operates over newly-active points.

(3) Keep the call in CNDriver, eliminate the call in clm_driver

For this to work, we'd need to have a new call to SoilBiogeochemVerticalProfile sometime (late) in initialization, so that these profiles were available to dyn_cnbal_patch in the first time step. That adds some extra complexity that isn't ideal.

This would change answers for transient cases with dyn_roots = .true., and I think would change answers for all transient cases due to the relative placement of alt_calc and SoilBiogeochemVerticalProfile. This means that a bit more analysis would be needed to make sure this is done correctly.

Other than that, this would remove the cons listed above.

[billsacks]

charlie < cdkoven@lbl.gov > - 2016-07-15 13:11:17 -0600

Bill,

I guess option 3 seems the most preferable long-term solution to me, but I'm not sure that it is sufficiently high priority to do this before the code freeze.

Thanks for thinking about this, Charlie