Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cfe_py is not completely compatible with bmi_py #20

Open
RY4GIT opened this issue Jul 16, 2023 · 3 comments
Open

cfe_py is not completely compatible with bmi_py #20

RY4GIT opened this issue Jul 16, 2023 · 3 comments

Comments

@RY4GIT
Copy link
Collaborator

RY4GIT commented Jul 16, 2023

cfe_py is not entirely convertible with the current version of bmi_py. Therefore, it returns an error when this model is plugged into Ngen.

I have debugged as much as possible, but I might have to leave due to time limitations as this is for this year.

Whoever wishes to plug in the CFE python version to NextGen software should ...

  • Start development from this branch add-bmi2 to avoid reinventing the wheel.
  • This is an example notebook bmi_cfe_py_experiment.ipynb where you can start experimenting

Changes made in add-bmi2 branch:

Use Bmipy package in bmi_py

from bmipy import Bmi
class BMI_CFE(Bmi):

Convert CFE as a package

  • Added setup.py
  • Create a folder bmi_py and put initialization file, bmi_cfe.py, and cfe.py under the folder

Fixed the get_var_type() function in bmi_py

Upon Nel's comments, I updated the get_var_type function (see discussion here NOAA-OWP/ngen/issues/571#issuecomment-1634818009)

Previous code:

    def get_var_type(self, long_var_name):
        """Data type of variable.

        Parameters
        ----------
        var_name : str
            Name of variable as CSDMS Standard Name.

        Returns
        -------
        str
            Data type.
        """
        # JG Edit
        return self.get_value_ptr(long_var_name)  #.dtype

This returned an error

terminate called after throwing an instance of 'std::runtime_error'
  what():  (Bmi_Py_Adapter) Failed determining analogous C++ type for Python model '0.0' type with size 4 bytes.

Updated code:

    def get_var_type(self, long_var_name):
        """Data type of variable.

        Parameters
        ----------
        var_name : str
            Name of variable as CSDMS Standard Name.

        Returns
        -------
        str
            Data type.
        """
        value_ptr = self.get_value_ptr(long_var_name)
    
        if isinstance(value_ptr, float):
            return "float"
        elif isinstance(value_ptr, np.ndarray):
            return str(value_ptr.dtype)
        else:
            return "Unknown"

Some suggestions for the developers

  • To try plugging in cfe_py to ngen, you should build ngen by yourself following this instruction. Before running cmake build, follow instruction here to enable debugging flags on.
  • CUAHSI-SI Jupyterhub or NextGen-in-a-Box doesn't give you a flexibility to plug in your own model.
  • I used example dataset downloaded from this tutorial (see the section Download the input data in "ngen-data" folder from S3 bucket :). It might be better to use any example data from updated hydrofabric outputs.
@RY4GIT
Copy link
Collaborator Author

RY4GIT commented Jul 16, 2023

@RY4GIT RY4GIT changed the title cfe_py is not completely convertible with bmi_py cfe_py is not completely compatible with bmi_py Jul 16, 2023
@RY4GIT
Copy link
Collaborator Author

RY4GIT commented Jul 17, 2023

Issue in variable name mapping (solved)

Still have issues in variable mapping.
I think this is an issue of pointing forcing variable names.

terminate called after throwing an instance of 'std::runtime_error'
  what():  bmi_py received invalid output forcing name APCP_surface

@RY4GIT RY4GIT pinned this issue Jul 20, 2023
@RY4GIT
Copy link
Collaborator Author

RY4GIT commented Jul 21, 2023

Issue in variable size passed to timestep_rainfall_input_m

The above issue was solved by matching up the ngen variable name and bmi_cfe for precipitation.

Precip variable name has been standardized as atmosphere_water__liquid_equivalent_precipitation_rate.

However, array of size (3) is passed to the cfe.py so that following error is returned.

terminate called after throwing an instance of 'pybind11::error_already_set'
  what():  ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

At:
  /home/jovyan/data/home/jovyan/cfe_py/bmi_cfe/cfe.py(27): calculate_evaporation_from_rainfall
  /home/jovyan/data/home/jovyan/cfe_py/bmi_cfe/cfe.py(211): run_cfe
  /home/jovyan/data/home/jovyan/cfe_py/bmi_cfe/bmi_cfe.py(316): update

By adding print(cfe_state.timestep_rainfall_input_m) before the line if(cfe_state.timestep_rainfall_input_m > 0): yields:
[4.17212082e-007 0.00000000e+000 1.56029587e-317]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant