Automatic running of internal evapotranspiration routines does not function as expected

[SnowHydrology]

Current behavior

When running a multi-BMI of the PET module with CFE, the framework returns a unit conversion error:

Unit conversion error:
Unable to convert mm s^-1 to kg m^-2
Returning unconverted value!

This should not happen because the updated PET module does not have precipitation in kg/m2 as part of its BMI spec (open PR here: https://github.com/NOAA-OWP/evapotranspiration/pull/10/files). If you run PET as a single module through Ngen, this error does not occur. This suggests something different is happening in the single vs. multi BMI. The following is what I believe the issue to be.

The model engine defines two standard variable names for potential evapotranspiration (

ngen/include/realizations/catchment/Bmi_Formulation.hpp

Line 35 in 0d04d52

#define NGEN_STD_NAME_POTENTIAL_ET_FOR_TIME_STEP "potential_evapotranspiration"

):

// Supported Standard Names for BMI variables
// This is needed to provide a calculated potential ET value back to a BMI model
#define NGEN_STD_NAME_POTENTIAL_ET_FOR_TIME_STEP "potential_evapotranspiration"

// Taken from the CSDMS Standard Names list
// TODO: need to add these in for anything BMI model input or output variables we need to know how to recognize
#define CSDMS_STD_NAME_POTENTIAL_ET "water_potential_evaporation_flux"

The engine also has logic to automatically run its own native evapotranspiration routines if a model in a formulation has one of those two standard names in its output variables (

ngen/include/realizations/catchment/Bmi_Module_Formulation.hpp

Line 374 in 1b79889

// Handle ET requests slightly differently

):

            // Handle ET requests slightly differently
            //TODO: This should really only happen if neither of these output_names are provided by the module...
            // But this code should also probably be removed in the near future (see GH #297 second comment).
            if (output_name == NGEN_STD_NAME_POTENTIAL_ET_FOR_TIME_STEP || output_name == CSDMS_STD_NAME_POTENTIAL_ET) {
                return calc_et();
            }

This has the effect of only running the native model engine evapotranspiration routines when the PET module is loaded in a multi BMI because it is the only module that has the CSDMS_STD_NAME_POTENTIAL_ET as an output var.

Expected behavior

The native evapotranspiration routines should not run when the PET module is used in a multi BMI. The issue is threefold:

1. The logic should run on the input vars if we keep it (i.e., it should search for modules that require PET, not those that produce it)
2. The logic should also check if there is a module in the stack producing the required PET (this is more complicated because plenty of modules may or will not use either of the standard names, meaning the number of options is undefined)
3. The PET module provides the same functionalities as the native evapotranspiration routines and for consistency we should prioritize the BMI-enabled module (there is nothing unique about PET as a flux that would suggest it should be handled differently than other modules)

Steps to replicate behavior (include URLs)

1. Run a multi-BMI formulation with PET and CFE

Interestingly, the error does not manifest when running Noah-OWP-Modular and CFE because the former does not use either standard names as its output var for PET.

[SnowHydrology]

Quick update. If you comment out lines 377-379 in include/realizations/catchment/Bmi_Module_Formulation.hpp and rebuild ngen, you can run PET in a multi-BMI without the framework running its own routines.