geoschem / HEMCO

The Harmonized Emissions Component (HEMCO), developed by the GEOS-Chem Support Team.
https://hemco.readthedocs.io
Other
15 stars 31 forks source link

"Too many nested brackets" error in HEMCO 3.8.0 #261

Closed yantosca closed 3 months ago

yantosca commented 4 months ago

Name and Institution (Required)

Name: Bob Yantosca Institution: Harvard + GCST

Confirm you have reviewed the following documentation

Description of your issue or question

While testing PR https://github.com/geoschem/geos-chem/pull/2171, I got this error in integration testing:

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%%%%%               HEMCO: Harmonized Emissions Component                 %%%%%
%%%%%               You are using HEMCO version 3.8.0                     %%%%%
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

Reading settings & switches of HEMCO configuration file: HEMCO_Config.rc

HEMCO verbose output is OFF

Reading fields of HEMCO configuration file: HEMCO_Config.rc

HEMCO ERROR: Too many nested brackets
 --> LOCATION: BracketCheck (hco_config_mod.F90)

HEMCO ERROR: Bracket error in HEMCO_Config.rc @ line (((.not.CEDS_01x01_SHIP
 --> LOCATION: Config_ReadCont (hco_config_mod.F90)

HEMCO ERROR: Error in HEMCO_Config.rc @ section: ### BEGIN SECTION BASE EMISSIONS
 --> LOCATION: Config_ReadFile (hco_config_mod.F90)

This is caused by the MAXBRACKET variable being set to 5 in HEMCO/src/Core/hco_config_mod.F90:

    ! Maximum number of nested brackets
    INTEGER, PARAMETER            :: MAXBRACKNEST = 5

I propose increasing this from 5 to a larger number such as 10. We may run into this in the future given that HEMCO does not recognize the .and. statement, but just .or. and .not..

lizziel commented 4 months ago

I thought that there is a limit to the number of nested brackets to avoid adding in excessive number of error checks, which can happen if the error check is deep in a calling stack. Before increasing the bracket we should probably figure out which error check is triggering the problem and make sure it is really needed. If it is deep in a series of subroutine calls then it is more likely be called in a loop.

yantosca commented 4 months ago

Thanks @lizziel. FWIW, the issue is happening here (see https://github.com/geoschem/geos-chem/blob/feature/ceds-0.1-degree/run/GCClassic/HEMCO_Config.rc.templates/HEMCO_Config.rc.fullchem).

(((CEDS_GBDMAPS_byFuelType
(((.not.CEDS_GBDMAPS
(((.not.CEDSv2
(((.not.CEDS_01x01
>>>include $ROOT/CEDS/v2020-08/HEMCO_Config.CEDS_GBDMAPS_byFuelType.rc
))).not.CEDS_01x01
))).not.CEDSv2
))).not.CEDS_GBDMAPS
)))CEDS_GBDMAPS_byFuelType
yantosca commented 4 months ago

Also in HEMCO the code that checks the brackets is here:

msulprizio commented 4 months ago

Is there a reason to keep CEDSv2 (i.e. the coarse-resolution version) in https://github.com/geoschem/geos-chem/pull/2171? That could help reduce the nesting in HEMCO_Config.rc?

yantosca commented 4 months ago

I could take it out @msulprizio. I wasn't sure if there's still a need for it.

yantosca commented 4 months ago

@msulprizio: the CEDSv2 data goes back to 1750 but the CEDS_01x01 goes back to 1980. I wasn't sure if people needed the historical data, which is why I thought to keep both.

lizziel commented 4 months ago

Ah, I am confusing this with a different max number of nests set in HEMCO.

msulprizio commented 4 months ago

@msulprizio: the CEDSv2 data goes back to 1750 but the CEDS_01x01 goes back to 1980. I wasn't sure if people needed the historical data, which is why I thought to keep both.

Thanks. I've commented on the original pull request and tagged the developers to see if we can reduce the number of CEDS options (and therefore nested brackets) in the default HEMCO_Config.rc.

yantosca commented 4 months ago

Thanks @msulprizio @lizzie. The reason we were hitting the max bracket was because of all the CEDS options.

yantosca commented 3 months ago

I thought that there is a limit to the number of nested brackets to avoid adding in excessive number of error checks, which can happen if the error check is deep in a calling stack. Before increasing the bracket we should probably figure out which error check is triggering the problem and make sure it is really needed. If it is deep in a series of subroutine calls then it is more likely be called in a loop.

@lizziel: I believe this is done in the initialization stage, when the config file is read. The end result is that a list of logicals is created that indicate if each bracket is activated or deactivated. So I think it doesn't add too much more overhead.

yantosca commented 3 months ago

We can now close this issue as PR #262 has been merged into the HEMCO "no-diff-to-benchmark" development stream.