Skip to content

Commit

Permalink
add options for GetPointer when a variabe is missing
Browse files Browse the repository at this point in the history
  • Loading branch information
@weiyuan-jiang
weiyuan-jiang committed Oct 5, 2022
1 parent 3d0c007 commit e5d4b65
Showing 1 changed file with 22 additions and 22 deletions.
44 changes: 22 additions & 22 deletions src/Components/GEOSldas_GridComp/GEOSens_GridComp/GEOS_EnsGridComp.F90
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2747,28 +2747,28 @@ subroutine Collect_land_ens(gc, import, export, clock, rc)
VERIFY_(status)


call MAPL_GetPointer(import, CNLAI , 'CNLAI' , _RC)
call MAPL_GetPointer(import, CNTLAI , 'CNTLAI', _RC)
call MAPL_GetPointer(import, CNSAI , 'CNSAI' , _RC)
call MAPL_GetPointer(import, CNTOTC , 'CNTOTC', _RC)
call MAPL_GetPointer(import, CNVEGC , 'CNVEGC', _RC)
call MAPL_GetPointer(import, CNROOT , 'CNROOT', _RC)
call MAPL_GetPointer(import, CNFROOTC , 'CNFROOTC', _RC)
call MAPL_GetPointer(import, CNNPP , 'CNNPP' , _RC)
call MAPL_GetPointer(import, CNGPP , 'CNGPP' , _RC)
call MAPL_GetPointer(import, CNSR , 'CNSR' , _RC)
call MAPL_GetPointer(import, CNNEE , 'CNNEE' , _RC)
call MAPL_GetPointer(import, CNXSMR , 'CNXSMR', _RC)
call MAPL_GetPointer(import, CNADD , 'CNADD' , _RC)
call MAPL_GetPointer(import, PARABS , 'PARABS', _RC)
call MAPL_GetPointer(import, PARINC , 'PARINC', _RC)
call MAPL_GetPointer(import, SCSAT , 'SCSAT' , _RC)
call MAPL_GetPointer(import, SCUNS , 'SCUNS' , _RC)
call MAPL_GetPointer(import, BTRANT , 'BTRANT', _RC)
call MAPL_GetPointer(import, SIF , 'SIF' , _RC)
call MAPL_GetPointer(import, CNLOSS , 'CNLOSS', _RC)
call MAPL_GetPointer(import, CNBURN , 'CNBURN', _RC)
call MAPL_GetPointer(import, CNFSEL , 'CNFSEL', _RC)
call MAPL_GetPointer(import, CNLAI , 'CNLAI' , notFoundOK=.true., _RC)

This comment has been minimized.

Sign in to view

Copy link
@gmao-rreichle

gmao-rreichle Oct 5, 2022

Contributor

@weiyuan-jiang, @gmao-jkolassa:
I think this should work, but I'm wondering if it's not better to be more explicit about the provenance of these imports. In GEOS_SurfaceGridComp.F90, we address this through "if LSM_CHOICE>1" and "if LSM_CHOICE==3" blocks (e.g., see in the line linked here)
Would such if blocks here be better because they provide more traceability for each variable? That is, is the variable in any version of Catchment and CatchmentCN, is it available only in CatchmentCNCLM4x, or only in CatchmentCNCLM45? @gmao-jkolassa is working on integrating CatchmentCNCLM5x, which probably has still more variables...
Or maybe we always need notFoundOK=.true.? If a variable is not in HISTORY.rc, it's not allocated as an export, and then the import here may not work. (But I never quite know how this works.) That is, the "if LSM_CHOICE..." blocks could be redundant, but maybe they would still provide clarity?
Thoughts?

This comment has been minimized.

Sign in to view

Copy link
@gmao-jkolassa

gmao-jkolassa Oct 6, 2022

Contributor

Here are my 2c:

Pros:

  • From a code-readability perspective it might make sense to include the if-blocks to make it clear which variables are present in all versions of Catchment-CN and which are specific to a certain version (or versions) of Catchment-CN (even if the code does not need this to run).
  • There will also most definitely be new version-specific variables that will be added for CatchmentCN5x, so there will be an increased number of variables that only "belongs" to one of the CN versions in the future
  • Having the if-blocks and removing the notFoundOK=.true may safeguard against potential issues with variables missing that should be present (although I do not know enough about the variable export to know whether this could ever be an issue)

Cons:

  • Having an if-block for each LSM_CHOICE could make this (and possibly other) parts of the code quite long and clunky, because of the need to repeat variables. For example, if a variable is in CN40 and CN45, but not in CN50, we would have to add the call MAPL_GetPointer statements to the if-blocks for both. This might also apply for other parts of the code if we decide to implement similar if-blocks there.

These are just my thoughts. Ultimately, I don't have a strong preference for either option.

This comment has been minimized.

Sign in to view

Copy link
@weiyuan-jiang

weiyuan-jiang Oct 6, 2022

Author Contributor

I prefer to remove "if" block. I actually have removed it in Land Gridcomp and CatchCN Grid comp. The "if" makes the code too clunky

This comment has been minimized.

Sign in to view

Copy link
@gmao-rreichle

gmao-rreichle Oct 6, 2022

Contributor

@weiyuan-jiang,
I suppose then we can just remove the "if LSM_CHOICE==..." blocks in the list of export specs in GEOS_SurfaceGridComp.F90 to be consistent. I would suggest replacing the "if ..." blocks with comments, e.g.,

! begin --- CatchCN-specific exports ---
[AddExportSpec CN...]
[AddExportSpec CN...]
! end   --- CatchCN-specific exports ---
! begin --- CatchCNCLM45-specific exports ---
[AddExportSpec CNFROOT goes here]
! end   --- CatchCNCLM45-specific exports ---

Such comments would help developers sort through the long lists.

I assume we can not use the new functionality MAPL_StateAddExportSpecFrmChld_All in GEOS_SurfaceGridComp.F90 because Surface is in grid space and the children are in tile space. Or could we?

This comment has been minimized.

Sign in to view

Copy link
@weiyuan-jiang

weiyuan-jiang Oct 6, 2022

Author Contributor

I think we could use the new MAPL's functionality. But would that be two many variables? Our GEOSldas does not use Surface GridCompanyway.
call MAPL_GetPointer(import, CNTLAI , 'CNTLAI', notFoundOK=.true., _RC)
call MAPL_GetPointer(import, CNSAI , 'CNSAI' , notFoundOK=.true., _RC)
call MAPL_GetPointer(import, CNTOTC , 'CNTOTC', notFoundOK=.true., _RC)
call MAPL_GetPointer(import, CNVEGC , 'CNVEGC', notFoundOK=.true., _RC)
call MAPL_GetPointer(import, CNROOT , 'CNROOT', notFoundOK=.true., _RC)
call MAPL_GetPointer(import, CNFROOTC , 'CNFROOTC', notFoundOK=.true., _RC)
call MAPL_GetPointer(import, CNNPP , 'CNNPP' , notFoundOK=.true., _RC)
call MAPL_GetPointer(import, CNGPP , 'CNGPP' , notFoundOK=.true., _RC)
call MAPL_GetPointer(import, CNSR , 'CNSR' , notFoundOK=.true., _RC)
call MAPL_GetPointer(import, CNNEE , 'CNNEE' , notFoundOK=.true., _RC)
call MAPL_GetPointer(import, CNXSMR , 'CNXSMR', notFoundOK=.true., _RC)
call MAPL_GetPointer(import, CNADD , 'CNADD' , notFoundOK=.true., _RC)
call MAPL_GetPointer(import, PARABS , 'PARABS', notFoundOK=.true., _RC)
call MAPL_GetPointer(import, PARINC , 'PARINC', notFoundOK=.true., _RC)
call MAPL_GetPointer(import, SCSAT , 'SCSAT' , notFoundOK=.true., _RC)
call MAPL_GetPointer(import, SCUNS , 'SCUNS' , notFoundOK=.true., _RC)
call MAPL_GetPointer(import, BTRANT , 'BTRANT', notFoundOK=.true., _RC)
call MAPL_GetPointer(import, SIF , 'SIF' , notFoundOK=.true., _RC)
call MAPL_GetPointer(import, CNLOSS , 'CNLOSS', notFoundOK=.true., _RC)
call MAPL_GetPointer(import, CNBURN , 'CNBURN', notFoundOK=.true., _RC)
call MAPL_GetPointer(import, CNFSEL , 'CNFSEL', notFoundOK=.true., _RC)



Expand Down

0 comments on commit e5d4b65

Please sign in to comment.