Improve mechanism for determining where to allocate memory for urban when running with dynamic urban #1661
Conversation
Also updated variable name from HASURBAN to PCT_URBAN_MAX (ESCOMP#1572 (comment)).
1. Fixed urban density type index for conditional initialization. 2. Change internal variable name from hasurban to pct_urban_max for consistency.
|
Thanks Bowen!
|
I also committed it to the inputdata repository
There was a problem hiding this comment.
This looks great - thanks a lot @fang-bowen and @olyson !
I pushed a small commit where I point to a new location for the landuse timeseries file in the smallville test: I have moved this to the inputdata repository.
See my one inline comment below, but I'll leave it up to you whether you want to change it or if you prefer to keep this more explicit code in subgridWeightsMod even if it means a bit of duplication.
Other than that, this is ready for final testing and integration.
src/main/subgridWeightsMod.F90
Outdated
| dens_index = lun%itype(l) - isturb_MIN + 1 | ||
| if (run_zero_weight_urban .or. pct_urban_max(g,dens_index) > 0._r8) then | ||
| is_active_l = .true. | ||
| end if |
There was a problem hiding this comment.
I'm not positive, but I think this could safely be changed to just return .true. for all urban landunits, similarly to what is done for lakes: this would say, if we have bothered to allocate memory for an urban landunit for any reason, then we want to make it active. I'm pretty sure that's what is intended here. However, the current code seems fine, too, so I'll leave it up to you whether you want to change it. The main advantages of changing it would be (1) consistency with lake, and (2) future-proofing in case the logic in subgridMod for when to allocate urban changes (currently the logic here needs to be kept consistent with the logic there).
There was a problem hiding this comment.
Actually, originally we had it set to just return .true. for all urban landunits , but that didn't seem to work in terms of not running an urban landunit that had a zero-weight throughout the transient time series. I checked this with the test I created in which one of the urban landunits was set to zero throughout the timeseries. If we simply returned .true. for all urban landunits here then the landunit would still show up in the history files as active with zero-weight. Using the statement we have now, the landunit no longer shows up.
I'll test this again to make sure this operates as I say.
There was a problem hiding this comment.
Given the current logic, I'm wondering why such an urban landunit would be allocated in the first place: if it has zero-weight throughout the time series, shouldn't the logic in urban_landunit_exists lead it to not getting allocated?
There was a problem hiding this comment.
Hmm, no, I think you are right. In both cases an ncdump of the h1 file gives:
data:
land1d_wtgcell = 1, 0, 0, 0 ;
land1d_ityplunit = 1, 2, 7, 8 ;
land1d_active = 1, _, 1, 1 ;
which is correct since the landuse.timeseries has zero percent urban for MD (land1d_ityplunit=9).
I agree that it would be good to be consistent, so I'll make the change you suggested.
There was a problem hiding this comment.
Sounds good, thanks for checking.
Allow user to replace vegetation in fsurdat files with any pft/cft using the fsurdat_modifier tool option dom_plant. This option replaces now obsolete option dom_nat_pft which handled pfts only and not cfts.
…nt with memory allocation in subgridMod.F90
|
Testing on cheyenne and izumi have passed as expected. Two expected failures on cheyenne due to changing the dynamic urban test: I've updated and committed the ChangeLog (please review). |
|
Looks great, thanks a lot! I'm about to merge this. (I just made a couple minor tweaks to the ChangeLog and ChangeSum; one is that I have started putting "multiple" in the ChangeSum when I'm bringing in a tag that had substantial contributions from multiple people, as this one seems to.) |
|
Thanks for the tweaks, good catches. |
Description of changes
Implement a 'pct_urban_max' mechanism that reads 'PCT_URBAN_MAX' (maximum urban percentage throughout timeseries) from landuse.timeseries and initialize urban landunits only where urban ever exists in the transient run.
Specific notes
As discussed in #1572, in the previous dynamic urban settings, the mechanism for allocating memory for urban is to set run_zero_weight_urban to .true., which causes performance and memory issues.
This PR includes the following changes to address the issue:
Add a subroutine surfrd_urbanmask to read 'PCT_URBAN_MAX' from landuse.timeseries, which indicates the maximum percentage of urban in the timeseries.
Add a 'pct_urban_max' variable and a function 'urban_landunit_exists'. If urban exists or will grow in a grid cell, memory is allocated.
Modify the urban logic in is_active_l. If memory is allocated for an urban column, then make it active.
Contributors other than yourself, if any:
@olyson. Also referenced #1109 ('haslake').
CTSM Issues Fixed (include github issue #): #1572
Are answers expected to change (and if so in what way)? No.
Any User Interface Changes (namelist or namelist defaults changes)?
No. For dynamic urban runs, user can leave 'run_zero_weight_urban' as .false. and supply landuse.timeseries file which includes PCT_URBAN_MAX (3 urban density types) and the model will only initialize urban landunits where urban exists or will grow.
Testing performed, if any:
Please see #1661 (comment) (Modified dynamic urban test; aux_clm testing).