UMEP-dev / SUEWS

Surface Urban Energy and Water Balance Scheme
https://suews.readthedocs.io/
Mozilla Public License 2.0
12 stars 8 forks source link

Problem with forcing interpolation when tstep=resolutionfilesin #161

Open RussellGl opened 1 year ago

RussellGl commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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. 1) 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 2) in the directory src/supy/supy, run make test and see if the current code with your new test can fail; 3) do the fix and make test again. 4) 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 commented 1 year ago

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 commented 1 year ago

Pushed the proposed changes to the issue161 fork.

Thanks, Russell

sunt05 commented 1 year ago

Pushed the proposed changes to the issue161 fork.

Thanks, Russell

Please submit a pull request - thanks.

sunt05 commented 4 months ago

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!