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.

Sign up for GitHub

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

Jump to bottom

pyet gives error during pet calculation penman-monteith #907

Open
2 tasks done
shartgring opened this issue Apr 24, 2024 · 5 comments · May be fixed by #918
Open
2 tasks done

pyet gives error during pet calculation penman-monteith #907

shartgring opened this issue Apr 24, 2024 · 5 comments · May be fixed by #918
Labels
Bug Something isn't working Needs refinement issue still needs refinement

Comments

@shartgring
Copy link

shartgring commented Apr 24, 2024 •
edited

HydroMT version checks

  • I have checked that this issue has not already been reported.
  • I have checked that this bug exists on the latest version of HydroMT.

Reproducible Example

Using simple Penman-Monteith method to determine pet results in an error. pyet is used to calculate the actual vapor pressure. The wrong arguments are parsed to the pyet function or the wrong function is used. From the hydromt log:

2024-04-24 20:45:43,668 - update - forcing - INFO - Calculating Penman-Monteith ref evaporation
2024-04-24 20:45:43,683 - update - main - ERROR - calc_e0() got an unexpected keyword argument 'tmax'
Traceback (most recent call last):
  File "C:\Users\hartgrin\miniconda\envs\hydromt-wflow\Lib\site-packages\hydromt\cli\main.py", line 332, in update
    mod.update(model_out=model_out, opt=opt, forceful_overwrite=fo)
  File "C:\Users\hartgrin\miniconda\envs\hydromt-wflow\Lib\site-packages\hydromt\models\model_api.py", line 329, in update
    self._run_log_method(method, **kwargs)
  File "C:\Users\hartgrin\miniconda\envs\hydromt-wflow\Lib\site-packages\hydromt\models\model_api.py", line 188, in _run_log_method
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\hartgrin\miniconda\envs\hydromt-wflow\Lib\site-packages\hydromt_wflow\wflow.py", line 2349, in setup_temp_pet_forcing
    pet_out = hydromt.workflows.forcing.pet(
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\hartgrin\miniconda\envs\hydromt-wflow\Lib\site-packages\hydromt\workflows\forcing.py", line 428, in pet
    pet_out = pm_fao56(
              ^^^^^^^^^
  File "C:\Users\hartgrin\miniconda\envs\hydromt-wflow\Lib\site-packages\hydromt\workflows\forcing.py", line 669, in pm_fao56
    avp = pyet.calc_e0(tmax=temp_max, tmin=temp_min, rh=temp_dew)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: calc_e0() got an unexpected keyword argument 'tmax'

And indeed, the following lines can be found in forcing.py:

    if var == "temp_dew":
        avp = pyet.calc_e0(tmean=temp_dew)
    elif var == "rh":
        avp = pyet.calc_e0(tmax=temp_max, tmin=temp_min, rh=temp_dew)

According to pyet documentation, pyet.calc_e0 takes only t_mean as argument. But maybe it should be pyet.calc_ea which calculated the actual vapor pressure? That one also accepts multitple arguments, including tmax, tmin, rh. See also https://github.com/pyet-org/pyet/blob/ff411787fbaeaa903b9966e28df41db9afb05a28/pyet/meteo_utils.py#L222

Note: I tried this using pyet version 1.3.1, hydromt version 0.9.4 and hydront_wflow version 0.5.0

Current behaviour

Gives an error when computing pet when not providing pressure term. The error is raised when pressure is calculated by pyet package

Desired behaviour

Change arguments for pyet.calc_e0 or use pyet.calc_ea

Additional context

No response
@shartgring shartgring added Bug Something isn't working Needs refinement issue still needs refinement labels Apr 24, 2024
@savente93
Copy link
Contributor

Hey @shartgring thank you for reporting this issue. The core team is really busy at the moment preparing for v1, so I'm not sure when we'll have time to address this issue, but given that you seem to have done some diagnosing yourself, would you be interesting in making a PR to fix this yourself? If you're not sure how to go about the PR I'm happy to walk you through it.

@shartgring
Copy link
Author

Sounds good Sam! I have made a fork as I needed the implementation this week. I will work a bit more on it and then see if I can share that. I'll come back to you if I encounter any problems or when I need some help :)

@shartgring shartgring linked a pull request May 6, 2024 that will close this issue
Open
6 tasks
@shartgring
Copy link
Author

@savente93 I fixed it on my own fork and tested it within my own project. Seems to work like a charm. I created a pull request #918 I hope the PR looks okay and otherwise let me know how I can improve it :)

@shartgring
Copy link
Author

The PET workflow using pyet can still be improved for readability as variables get mixed up. However in terms of fixing that the error is not raised anymore, the fix in the PR is sufficient

@savente93
Copy link
Contributor

I'll try and have a look at it today. thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Something isn't working Needs refinement issue still needs refinement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Bugfix when using pyet method to calculate PET shartgring/hydromt
2 participants
@savente93 @shartgring