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

Problem with forcing interpolation when tstep=resolutionfilesin #161

Open
RussellGl opened this issue Feb 20, 2023 · 9 comments
Open

Problem with forcing interpolation when tstep=resolutionfilesin #161

RussellGl opened this issue Feb 20, 2023 · 9 comments
Assignees
Labels
P1 High priority (should be addressed soon)

Comments

@RussellGl
Copy link

In SuPy, when the time step of the model is equal to the time resolution of the input data going into making the forcing for the model, the model interpolates the forcing data when no interpolation is required. The pieces of code which are involved in this interpolation step are in '_load.py' and in the 'resample_forcing_met' function. In this function the forcing data is shifted in such a way that when the interpolation step comes, it will interpolate between the current time and 1 timestep ahead. Since tstep and the deltat of the forcing are the same no interpolation is necessary. These interpolation functions seem to assume that 'resolutionfilesin' is larger than 'tstep' and the current method seems to work correctly when this condition is true.

A test tracking the 'Kdown' values was performed to find where incorrect interpolation originates. The line is in 'resample_forcing_met' function:

data_met_tstep = (
    pd.concat([data_met_tstep_inst, data_met_tstep_avg, data_met_tstep_sum], axis=1)
    .interpolate()
    .loc[data_met_tstep_inst.index]
)

Only variables which require averaging seem to be affected by this and other instantaneous variables like 'Tair' are not affected and the forcing values are the same as the input file provided by the user.

Recommend to add a condition when tstep=resolutionfilesin to skip this interpolation section and directly make the forcing using the input file provided by the user.

@sunt05 sunt05 self-assigned this Feb 20, 2023
@sunt05
Copy link

sunt05 commented Feb 20, 2023

Thanks @RussellGl !
there are several reorganisation tasks going on to properly merge supy into SUEWS - once it's done, I'll add the option.

@suegrimmond
Copy link

Propose @RussellGl has a go at making this fix and submitted the code back to GitHub - to start to learn about developing code with GitHub
ok @sunt05 ?

@sunt05
Copy link

sunt05 commented Feb 21, 2023

Sounds good!

I'm merging supy code with SUEWS at the moment - some tests to be done.
Once I finish the merge for this stage, @RussellGl I'll let you know and may show you how to use the GH pull request based workflow for the development.

@sunt05
Copy link

sunt05 commented Feb 21, 2023

@RussellGl preferably, please set up your VScode with GitHub pull request plugin set up: https://marketplace.visualstudio.com/items?itemName=GitHub.vscode-pull-request-github

@sunt05
Copy link

sunt05 commented Feb 22, 2023

Hi @RussellGl , the supy code has been merged into suews as one set of codebase now.
Please use the master branch to create a new branch and give it a try to fix this issue.
The supy code is here:
https://github.com/UMEP-dev/SUEWS/tree/master/src/supy/supy

When you do the fix, please follow the steps below:

  1. create a conda env using the env.yml file: recommend using mamba instead of conda.
  2. before fix the code, please write a test first here following other examples:
    https://github.com/UMEP-dev/SUEWS/blob/master/src/supy/supy/test/test_SuPy.py
  3. in the directory src/supy/supy, run make test and see if the current code with your new test can fail;
  4. do the fix and make test again.
  5. if all tests can be passed, clean up and submit a PR.

Let me know if anything unclear and help is needed - I'll be available today after 3:30pm and can have a call if needed.
Thanks!

@RussellGl
Copy link
Author

Hi Ting,

Okay I think it would be good to have a call in the afternoon, it is hard for me to follow.

Thanks a lot

@RussellGl
Copy link
Author

Pushed the proposed changes to the issue161 fork.

Thanks,
Russell

@sunt05
Copy link

sunt05 commented Feb 28, 2023

Pushed the proposed changes to the issue161 fork.

Thanks, Russell

Please submit a pull request - thanks.

@sunt05
Copy link

sunt05 commented Jun 8, 2024

Pushed the proposed changes to the issue161 fork.

Thanks, Russell

Hi @RussellGl I seem not to be able to find the fork you mentioned here - can you please give me a link? thanks!

@sunt05 sunt05 added the P1 High priority (should be addressed soon) label Jun 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 High priority (should be addressed soon)
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

3 participants