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

UWG error if soil depth < 3 in EPW file. #96

Open
saeranv opened this issue Dec 4, 2018 · 9 comments
Open

UWG error if soil depth < 3 in EPW file. #96

saeranv opened this issue Dec 4, 2018 · 9 comments

Comments

@saeranv
Copy link
Member

saeranv commented Dec 4, 2018

@hansukyang,

One of our users ran into trouble with an epw file that provided ground temperatures for only one soil depth, and not three as we have tested in the past.

Specifically, the monthly self.forc.waterTemp variable is taken from the 3rd soil depth in all cases, unless the number of soil depths is equal to zero (this is how the original matlab code treats this).

          if self.is_near_zero(self.nSoil):
                # for BUBBLE/CAPITOUL/Singapore only
                self.forc.deepTemp = sum(self.forcIP.temp)/float(len(self.forcIP.temp))
                self.forc.waterTemp = sum(
                    self.forcIP.temp)/float(len(self.forcIP.temp)) - 10.      # for BUBBLE/CAPITOUL/Singapore only
            else:
                # soil temperature by depth, by month
                self.forc.deepTemp = self.Tsoil[self.soilindex1][self.simTime.month-1]
                self.forc.waterTemp = self.Tsoil[2][self.simTime.month-1]

I changed the code so that the if condition is now if self.nSoil < 3: , that is, if any soil depth less then three is provided, the uwg will default to taking the self.forc.waterTemp, and self.forc.deepTemp using the method it uses for when no soil depth is provided in the uwg.

My changes are here: https://github.com/ladybug-tools/uwg/pull/95/files#diff-a226d4381c852ce81325ea5b7180fdc2R772

Let me know if this sounds okay, and if not, I'll revert the code.

Thank you,

Saeran

@hansukyang
Copy link

hansukyang commented Dec 5, 2018 via email

@saeranv
Copy link
Member Author

saeranv commented Dec 6, 2018

@hansukyang

I see, in that case, I agree we should try and calculate the water temperature via a thermal diffusion routine. I won't be the person to write it, but can ask a work colleague to take a look at it, and will run it by you.

As for the treeSensHeat question, I understand it's tricky! The heat contribution of the trees and vegetation are split across a couple of different methods (SolarCalcs, urbflux, infracalcs, element.surfFlux) so it's understandable it's harder to pin down. It's not an urgent problem, so we can let it be if we need to.

By the way, that question came out of a user's questions about the vegetation/tree participation in the UWG heat balance from here: https://discourse.ladybug.tools/t/general-questions-about-the-input-parameters/4358 and inexplicable behaviour of trees/vegetation here: https://discourse.ladybug.tools/t/urban-heat-island-in-an-all-green-scenario/3497/6

Feel free to jump into those discussions and answer directly.

The latter question, regarding tree obstruction of IR exchange, deserves a git issue of it's own I think, since after reviewing the code, and reading Bruno's original chapter on the objective of the vegetation in UWG, it still doesn't make sense to me why the tree fraction is not part of the IR exchange, and conversely, why the direct and diffuse solar received by the ground isn't blocked by trees. Again, not urgent issues, but worth documenting to see if I'm just missing something obvious here.

S

@hansukyang
Copy link

hansukyang commented Dec 7, 2018 via email

@saeranv
Copy link
Member Author

saeranv commented Nov 13, 2020

Update: I actually have more on my plate then expected so am adding a "help wanted" tag to this issue. If anyone wants to tackle this, go for it!

@hansukyang
Copy link

I added soil temperature to the weather API for cases such as this (reference). Not sure how that would help in this usage case though but is this something that gets raised often?

@saeranv
Copy link
Member Author

saeranv commented Nov 15, 2020

@hansukyang

I would estimate the issue is raised rarely, since we had the UWG released for a while before it finally threw the error.

Does your weather API calculate the soil temperatures at different depths using the thermal diffusion routine?

@hansukyang
Copy link

Soil temperatures at various depths is already calculated by the numerical weather models (both ERA5 and GFS) that the data is sourced from. This would again be a much more sophisticated version of the thermal diffusion routine since it would take into account soil type, soil moisture content, water run-off etc.

@saeranv
Copy link
Member Author

saeranv commented Nov 16, 2020

@hansukyang

If the reanalysis data is calculating the soil temperatures more accurately then the calculation we were discussing more, then should we just recommend users use reanalysis data if they care about modeling the ground and water temperature accurately in scenarios with missing soil depth temperatures?

Why increase the code complexity with a suboptimal calculation if a more accurate alternative exists?

I can add a warning when soil temps are less then 3, with an explanation that we are using the average air temp minus 10, but that we recommend weather files with at least 3(?) soil depths for more accurate ground and water temperatures.

@hansukyang
Copy link

Yeah that would make sense. This would be especially the case if they are trying to use AMY.

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

No branches or pull requests

2 participants