max_patch_per_column and loop control
I started this thread because I thought max_patch_per_column is larger than it needs to be and is causing some inefficiency in the land model. But after reading through where it is currently used, I think the bigger issue is about how it is used and how inefficient it is (unless I'm missing something obvious).
It is used to perform initialization calls to Betr and MPP. @jinyuntang @bishtgautam and I defer to their thoughts on how this is used.
Initialization for MPP: https://github.com/E3SM-Project/E3SM/blob/master/components/elm/src/main/elm_initializeMod.F90#L1064
Initialization for Betr: https://github.com/E3SM-Project/E3SM/blob/master/components/elm/src/main/elm_initializeMod.F90#L627
Here is where it is set: https://github.com/E3SM-Project/E3SM/blob/master/components/elm/src/main/elm_varpar.F90#L145
Note in that definition, that numpft is really the number of natural+crop pfts. If you are running with use_crops you get this: numpft = 50 numcft = 36
without use_crops: numpft = 16 numcft = 2
The number of PFTs you get on the column though, which I believe is what this index is supposed to signal, should be the maximum of: natpft_size, cft_size , maxpatch_urb
Here is an example of where it is used in the ELM code (note all of its uses, aside from intializing MPP and betr, are for executing the same looping behaviour seen here): https://github.com/E3SM-Project/E3SM/blob/master/components/elm/src/biogeophys/SoilWaterMovementMod.F90#L1203-L1208
My interpretation is that we should change this so that we loop from pfti to pftf, inside the column loop.
Other locations used: https://github.com/E3SM-Project/E3SM/blob/master/components/elm/src/biogeophys/SoilFluxesMod.F90#L233-L237 https://github.com/E3SM-Project/E3SM/blob/master/components/elm/src/biogeophys/SoilTemperatureMod.F90#L1775-L1779 https://github.com/E3SM-Project/E3SM/blob/master/components/elm/src/biogeochem/FireMod.F90#L303-L307 https://github.com/E3SM-Project/E3SM/blob/master/components/elm/src/biogeochem/CNPhenologyBeTRMod.F90#L3023-L3029 https://github.com/E3SM-Project/E3SM/blob/master/components/elm/src/biogeochem/CarbonIsoFluxMod.F90#L865-L869
Perhaps the next question is asking in these above use cases, what is that logic trying to achieve? Is the objective to loop through every patch on the column? Or is the object to loop through a filter set of patches on the column? There are more efficient ways to do the latter right?
also tagging @thorntonpe @peterdschwartz
@rgknox I recalled I had the same confusion too, but never tried to solve it systematically even in betr. Using pfti to pftf as loop controls definitely improves the efficiency.
Yes, this is inefficient -- I'm guessing since the code was written before the filters were implemented. (Like how all local arrays are allocated based on the bounds rather than the num_filter). I've changed a couple of these that are in master already, for instance PhenologyMod::CNLitterToColumn just loops over the patch filter and uses veg_pp%column to get the column. If it's possible to just replace in this manner, then that's the most efficient method for CPU code I believe.
For GPU porting reasons, I've predominantly kept the column filter and use pfti and pftf as you suggest to take advantage of the vectorized reduction operations, for what it's worth.
@jinyun1tang , regarding the value that is passed to betr, is it supposed to be the maximum possible number of patches on a column, or something else?
Further, which LU's does betr run on? I assume the natural landunit.. crop, urban, etc?
@rgknox , yes it is the maximum possible number of patches on a column, so memory is properly allocated. Now betr only runs over natural land unit, or soil column (bare soil + natural pft).
In that case, I think natpft_size is what I would pass in to betr then. If you started to run it on the crop LU as well, then the maximum of natpft_size and cft_size
Sounds a good plan.