search

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

Implementation of code from v2024.3.5.dev to current version #283

Open MatthewPaskin opened 2 months ago

MatthewPaskin commented 2 months ago

Describe the Issue Some code for coupling a building energy model was added to version v2024.3.5.dev as can be found here: https://github.com/Urban-Meteorology-Reading/SUEWS-STEBBS

I am looking to make this same implementation in the newest version but significant changes have been made to the file SUEWS_ctrl_driver.f95 and I am struggling to identify where to make this new implementation.

Additional context I am unable to compile the v2024.3.5.dev version due to an issue with numpy.distutils failing to import distutils.msvccompiler after make dev is called, despite this being available to my python environment.

sunt05 commented 2 months ago

@MatthewPaskin, could you please grant me access to the SUEWS-STEBBS repository?

suegrimmond commented 2 months ago

@MatthewPaskin, could you please grant me access to the SUEWS-STEBBS repository?

done

sunt05 commented 2 months ago

@MatthewPaskin, could you direct me to the STEBBS code for coupling with SUEWS? I see there are several Fortran files. I'll provide guidance on the coupling.

Additional context I am unable to compile the v2024.3.5.dev version due to an issue with numpy.distutils failing to import distutils.msvccompiler after make dev is called, despite this being available to my python environment.

You may encounter such issue in a native Windows environment because the make dev command uses buildwheel to build supy. This process creates its own virtual environment using the official Python release, which has several known issues with the MSVC compiler due to poor maintenance of distutils, now semi-deprecated. Fixing this is torturing—I struggled significantly when addressing it for a Windows environment with distutil in previous supy releases. This experience strongly motivated me to switch to the recently introduced mason-based building process for supy.

I recommend using WSL for supy development on Windows. It's essentially a Linux environment, and I can help you better if any issues arise.

MatthewPaskin commented 2 months ago

@MatthewPaskin, could you direct me to the STEBBS code for coupling with SUEWS? I see there are several Fortran files. I'll provide guidance on the coupling.

The main file that I have found a difference is in ./SUEWS/src/suews/src/suews_ctrl_driver.f95. It seems the structure of this has significantly changed. The notes on the implementation are as follows:

@l.2914

!nakao b
#ifndef STEBBSONLINE
#else
!
call stebbsonlinecouple(tstep, [                                 &
                     REAL(iy, KIND(1D0)), REAL(id, KIND(1D0)),   &
                     REAL(it, KIND(1D0)), REAL(imin, KIND(1D0)), &
                     dectime],                                   &
                               T2_C, Tsurf,                      &
                               dataOutLineBEERS(6),              & ! Kdown
                               0.25d0*(dataOutLineBEERS(8)  +    & ! Kwall(SWNE)
                                       dataOutLineBEERS(9)  +    &
                                       dataOutLineBEERS(10) +    &
                                       dataOutLineBEERS(11) ),   &
                               0.25d0*(dataOutLineBEERS(14) +    & ! Lwall(SWNE)
                                       dataOutLineBEERS(15) +    &
                                       dataOutLineBEERS(16) +    &
                                       dataOutLineBEERS(17) ),   &
                               dataOutLineBEERS(12)          ,   & ! Ldown2d
                               U10_ms )
#endif
!nakao a

My issue is just finding an equivalent in the new file as SUEWS_calc_main_DTS is no longer present in the file.

You may encounter such issue in a native Windows environment because the make dev command uses buildwheel to build supy. This process creates its own virtual environment using the official Python release, which has several known issues with the MSVC compiler due to poor maintenance of distutils, now semi-deprecated. Fixing this is torturing—I struggled significantly when addressing it for a Windows environment with distutil in previous supy releases. This experience strongly motivated me to switch to the recently introduced mason-based building process for supy.

I recommend using WSL for supy development on Windows. It's essentially a Linux environment, and I can help you better if any issues arise.

I am actually working on a Linux system but I am using pip to keep a cleaner python installation (rather than having multiple sources e.g. with python and conda). I have tried on my home system (Arch) and through WSL (Ubuntu) on my windows laptop and had the same issues each time.

sunt05 commented 2 months ago

Hi @MatthewPaskin, let's park the dev env issue for now—as long as you're using Linux, that's great.

Regarding the source code, which STEBBS file should I refer to? There are several in the repository. Please post a link to the file.

MatthewPaskin commented 2 months ago

I believe the STEBBS file to be added to SUEWS/src/suews/src is the STEBBS_f95dev/src/suews_phys_stebbs.f95 file.

sunt05 commented 2 months ago

This one?

https://github.com/Urban-Meteorology-Reading/SUEWS-STEBBS/blob/main/STEBBS_f95dev/src/suews_phys_stebbs.f95

MatthewPaskin commented 2 months ago

yes although on compilation the gocreate.sh (found in STEBBS_f95dev/sh/) modifies this file slightly e.g. on line 11.

sunt05 commented 2 months ago

Hi @MatthewPaskin,

I’m back from annual leave and have reviewed the STEBBS code. I noticed it uses a file-based approach for input and output with the main SUEWS code, writing results as external files.

To make the coupled code more efficient and easier to maintain, I suggest the following:

  1. Refactor the STEBBS code to include explicit argument lists for input and output variables. This will help me assist you better with coupling.

  2. Use the RSL code in SUEWS as an example: https://github.com/UMEP-dev/SUEWS/blob/55ff2d95374a712b71d7945e90a4fd54cea6a53f/src/suews/src/suews_phys_rslprof.f95#L760-L763

Additionally, please use the Fortran extension in VS Code for navigation between definitions and references during development.

Any questions, please let me know.

sunt05 commented 2 months ago

I've created a branch for this work: https://github.com/UMEP-dev/SUEWS/tree/sunt05/issue283.

The STEBBS source code has been copied from your repo: https://github.com/UMEP-dev/SUEWS/blob/sunt05/issue283/src/suews/src/suews_phys_stebbs.f95#L0-L1

Let's collaborate there.

MatthewPaskin commented 2 months ago

Hi @sunt05 , many thanks for taking a look at this. I will take a look at your suggestions and will keep in contact regarding this coupling.

I noticed last week I am unable to commit to branches on this repository. Would it be possible to enable commits for this new branch?

sunt05 commented 2 months ago

Hi @MatthewPaskin, I've added you to the SUEWS-dev team. You can now commit to the SUEWS repository.

MatthewPaskin commented 2 months ago

@sunt05, I think the code can either use a file approach but the subroutine stebbsonlinecouple looks as though it is already in the form you suggest.

https://github.com/UMEP-dev/SUEWS/blob/dca7fa2852c4d94757e4327f75006302a67d96da/src/suews/src/suews_phys_stebbs.f95#L669-L670

sunt05 commented 2 months ago

Yes, I noticed this subroutine too, and it seems to be where the coupling could be improved.

Additionally, examining the code at

https://github.com/UMEP-dev/SUEWS/blob/dca7fa2852c4d94757e4327f75006302a67d96da/src/suews/src/suews_phys_stebbs.f95#L885

shows that the output is written as external files: https://github.com/UMEP-dev/SUEWS/blob/dca7fa2852c4d94757e4327f75006302a67d96da/src/suews/src/suews_phys_stebbs.f95#L921-L940

These could be stored explicitly as arrays following the SUEWS convention mentioned in my earlier post.

MatthewPaskin commented 2 months ago

Hi @sunt05. Part of the STEBBS module uses the radiation onto the building walls from BEERS which is separated by the cardinal wall directions, however STEBBS only takes a single value for wall radiation. This was previously processed externally in python and fed into STEBBS.

Would you advise that in the new fortran module I calculate this value within the STEBBS f95 file after passing the BEERS output, or instead use python or calculate in a different f95 file before passing to the STEBBS file? Thanks

sunt05 commented 2 months ago

Hi @MatthewPaskin, let's do this:

in the new fortran module I calculate this value within the STEBBS f95 file after passing the BEERS output