ladybug-tools / uwg

:city_sunrise: The Urban Weather Generator (uwg) is a Python application for modeling the urban heat island effect.
https://www.ladybug.tools/uwg/docs/
GNU General Public License v3.0
52 stars 25 forks source link

Revise time-step calculation in UWG #3

Open saeranv opened 7 years ago

saeranv commented 7 years ago

Errors are appearing in UWG due to explicit time-step calculation that should be changed to implicit. Copying and pasting time-step revision discussion by @hansukyang for reference:

"....in Building.m and UCMDef.m, there are two parts that were simplified, essentially assuming that the air temperature of a control volume is a weighted average of its surface boundary temperature, with each temperature value weighted by its area and the heat transfer coefficient, and adding in the heat generated. So for instance (from building.m):

        Q = obj.intHeat + winTrans + obj.Qheat-obj.sensCoolDemand;
        H1 = T_wall*wallArea*zac_in_wall + T_mass*massArea*zac_in_mass + T_ceil*zac_in_ceil + T_can*winArea*obj.uValue + T_can*volInfil * dens * parameter.cp + T_can*volVent * dens * parameter.cp;               
        H2 = wallArea*zac_in_wall + massArea*zac_in_mass + zac_in_ceil + winArea*obj.uValue + volInfil * dens * parameter.cp + volVent * dens * parameter.cp;
        obj.indoorTemp = (H1 + Q)/H2; <- this part here is the weighted averaging

where you can see how each term is summed. This doesn’t quite work for the urban boundary layer and solid element layers because the above assumes that within the time step (~5 min or so), the temperature within the control volume has equalized. Given the rate of change of other time-dependant parameters that changes its value every hour or so, this was assumed to be sufficient. "

chriswmackey commented 7 years ago

@saeranv . Thanks again for posting this.

I discovered something new about this bug today, which I am going to paraphrase at the "FATAL ERROR" bug. It seems to be affected both by the thicknesses of materials as @hansukyang mentioned, as well as the weather file.

For example, I can run this hydra example file for the whole year using the Singapore weather file: http://hydrashare.github.io/hydra/viewer?owner=chriswmackey&fork=hydra_2&id=Urban_Weather_Generator_Workflow

However, I cannot run it for the full year with the Miami Weather File. If I manage to get @hansukyang's thoughts on this, I will post it here.

saeranv commented 6 years ago

@hansukyang

Do you know what the specific condition is that triggers the "FATAL ERROR" message in this code?

% Indoor convection heat transfer coefficients
zac_in_wall = 3.076;
zac_in_mass = 3.076;
if (T_ceil > T_indoor)
     zac_in_ceil  = 0.948;
elseif(T_ceil <= T_indoor);
     zac_in_ceil  = 4.040;
else
     disp('!!!!!FATAL ERROR!!!!!!');
return;
end

I ask because this particular conditional expression doesn't translate well to python. It ends up looking like this:

if T_ceil > T_indoor:                           # set higher ceiling heat convection coefficient
     zac_in_ceil  = 0.948                        # - based on heat is higher on ceiling
elif (T_ceil < T_indoor) or self.is_near_zero(T_ceil-T_indoor):
     zac_in_ceil  = 4.040
 else:
     print T_ceil, T_indoor
     raise Exception(self.TEMPERATURE_COEFFICIENT_CONFLICT_MSG)
 return

...which I'm pretty sure will never get triggered in Python. (T_ceil should always be > or <= T_indoor, the else condition should never occur).

Unfortunately I'm unable to reproduce, and test this FATAL ERROR bug on my versions of Matlab or Octave, (in which case I might have been able to figure this out myself). Long story short - the read .xml method in Octave and Matlab are failing for me, and that so far is the only input method where I've encountered this bug.

Do you have any idea of what we should be writing in python to catch this error?

Thanks again,

-S

hansukyang commented 6 years ago

Hi Saeran,

The fatal messasge is triggered during an unbounded error, i.e. when integration fails to converge, which happens when a layer is too thin or thermal conductivity too high. It's a slightly odd way of catching this type of errors (usually when it's too late) so it can be done in other improved matters.

With regards to other questions:

  1. I don't think latAnth and latTree were used anywhere but left in for possible future applications if I remember correctly.
  2. This was added by me and I think this was carried over from the DOE reference building input. I don't think I implemented this fully.

Let me know if this helps - the details are somewhat vague so I'll have to go back and take a look at the code to answer in more detail so let me know if you need more clarification.

Joseph

On 27 May 2018 at 15:41, Saeran Vasanthakumar notifications@github.com wrote:

@hansukyang https://github.com/hansukyang

Do you know what the specific condition is that triggers the "FATAL ERROR" message in this code?

% Indoor convection heat transfer coefficients zac_in_wall = 3.076; zac_in_mass = 3.076; if (T_ceil > T_indoor) zac_in_ceil = 0.948; elseif(T_ceil <= T_indoor); zac_in_ceil = 4.040; else disp('!!!!!FATAL ERROR!!!!!!'); return; end

I ask because this particular conditional expression doesn't translate well to python. It ends up looking like this:

if T_ceil > T_indoor: # set higher ceiling heat convection coefficient zac_in_ceil = 0.948 # - based on heat is higher on ceiling elif (T_ceil < T_indoor) or self.is_near_zero(T_ceil-T_indoor): zac_in_ceil = 4.040 else: print T_ceil, T_indoor raise Exception(self.TEMPERATURE_COEFFICIENT_CONFLICT_MSG) return

...which I'm pretty sure will never get triggered in Python. (T_ceil should always be > or <= T_indoor, the else condition should never occur).

Unfortunately I'm unable to reproduce, and test this FATAL ERROR bug on my versions of Matlab or Octave, (in which case I might have been able to figure this out myself). Long story short - the read .xml method in Octave and Matlab are failing for me, and that so far is the only input method where I've encountered this bug.

Do you have any idea of what we should be writing in python to catch this error?

Thanks again,

-S

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ladybug-tools/urbanWeatherGen/issues/3#issuecomment-392311515, or mute the thread https://github.com/notifications/unsubscribe-auth/ALUG7XGZqmAB1PvSiJ8aSa6X4wYNNO4wks5t2liVgaJpZM4NFIGq .

On 27 May 2018 at 15:00, Saeran Vasanthakumar notifications@github.com wrote:

@hansukyang https://github.com/hansukyang

I just added the optional parameters into the UWG https://github.com/ladybug-tools/urbanWeatherGen/blob/master/UWG/UWG.py#L524-L530 and wanted to clarify a few things before I close this issue.

And, for reference, this is how our initialize file looks like right now: https://github.com/ladybug-tools/urbanWeatherGen/blob/ master/resources/parameters/initialize_singapore.uwg

1.

Regarding your removal of certain latent heat calculations: are there any references to latent heat in our initialize.uwg file that are not being used in the simulation? We want to make sure to inform users if any of these parameters aren't being used in the engine. Specifically, I can tell latGrss is being used to calculate surface flux on the elements with vegetation, and that LatFOcc is being used to calculate building internal loads. However I'm not so sure if latAnth, and latTree are being used. They are added to the UCM.latheat (urbflux.py line 48) but then I don't see the UCM.latheat variable used anywhere. 2.

For our optional parameter of the hvac: hvac, , # HVAC TYPE; 0 = Fully Conditioned (21C-24C); 1 = Mixed Mode Natural Ventilation (19C-29C + windows open >22C); 2 = Unconditioned (windows open >22C) Is this being used anywhere? Or is this something that is a hold-over from an older version of UWG?

Thanks for being so generous with your help! This whole project would be a lot more complicated without your input. Let me know if you need further clarification.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ladybug-tools/urbanWeatherGen/issues/18#issuecomment-392309705, or mute the thread https://github.com/notifications/unsubscribe-auth/ALUG7TcE8hkZVm6dvG-EtOm_JJ4tLFJDks5t2k8kgaJpZM4Px6ld .

saeranv commented 6 years ago

@hansukyang

Sorry for my late response. Thanks for clarifying the cause of the error, its helping illuminate the problem. However I'm still confused about how practically speaking the Python code should be written to catch this error.

If you look at this pseudo-code version of the FATAL ERROR condition, it's basically running this:

if T_ceil > T_indoor:
   do_something          
elif (T_ceil <= T_indoor):
   do_something
else:
     raise "FATAL ERROR"

... or to put it in english - If ceiling temperature neither greater then, less then, or equal to indoor temperature, raise the "FATAL ERROR". Maybe I'm missing something obvious here, but when this is translated to Python, as long as T_ceil, and T_indoors always returns a float, they will always meet one of those conditions, and therefore produce the "FATAL ERROR" message, even if the unbounded error is encountered. And based on the code, T_ceil, and T_indoors will always return a float.

And if this is the case, what should we be writing in this case, in Python, to catch the unbounded error. I hope my question is a bit clearer now. Let me know if you need more details.

And now that I have a better sense of what the error is, I'll do some research on my end on how best to deal with convergence/unbounded errors (???) in Python.

S

hansukyang commented 6 years ago

I think something like below? I'm not sure how Python and Matlab's way of handling infinite numbers (i.e. 'NaN') are different, but this would be one way.

try: if T_ceil > T_indoor: do_something elif (T_ceil <= T_indoor): do_something except ValueError: (or TypeError) raise "FATAL ERROR"

Another, perhaps more robust method might be to check that T_ceil & T_indoor are always within reasonable bounds - say between -50C to 100C.

Exceeding these limits occur just before the integration becomes unbounded so it'll catch the error just slightly early with the same effect, and might help make the code a bit more clearer.

It took me a while to understand what this part of the code was doing as well, so perhaps some clarifications can help. Let me know if this clarifies your question.

Joseph

On 10 June 2018 at 14:50, Saeran Vasanthakumar notifications@github.com wrote:

@hansukyang https://github.com/hansukyang

Sorry for my late response. Thanks for clarifying the cause of the error, its helping illuminate the problem. However I'm still confused about how practically speaking the Python code should be written to catch this error.

If you look at this pseudo-code version of the FATAL ERROR condition, it's basically running this:

if T_ceil > T_indoor: do_something elif (T_ceil <= T_indoor): do_something else: raise "FATAL ERROR"

... or to put it in english - If ceiling temperature neither greater then, less then, or equal to indoor temperature, raise the "FATAL ERROR". Maybe I'm missing something obvious here, but when this is translated to Python, as long as T_ceil, and T_indoors always returns a float, they will always meet one of those conditions, and therefore produce the "FATAL ERROR" message, even if the unbounded error is encountered. And based on the code, T_ceil, and T_indoors will always return a float.

And if this is the case, what should we be writing in this case, in Python, to catch the unbounded error. I hope my question is a bit clearer now. Let me know if you need more details.

And now that I have a better sense of what the error is, I'll do some research on my end on how best to deal with convergence/unbounded errors (???) in Python.

S

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ladybug-tools/urbanWeatherGen/issues/3#issuecomment-396025362, or mute the thread https://github.com/notifications/unsubscribe-auth/ALUG7S_Z-32BAWsoeIk47OER98sERrW7ks5t7MG0gaJpZM4NFIGq .

saeranv commented 6 years ago

@hansukyang thanks, that's exactly what I was looking for!

I just edited the code here: https://github.com/ladybug-tools/urbanWeatherGen/blob/ea6695dbcaa3f188a12a67b94ff067ade5c087ac/UWG/building.py#L179-L199

I'm also not sure if a NaN (or something else) will be generated in python in this case, so I added a try/except loop, for the temperature bound checking.

We'll still have to figure out how to solve this error, but at least now I'm confident we'll catch it when it occurs.

hansukyang commented 6 years ago

Great to hear.

For this error not to occur, the heat conduction calculation will likely have to be implicitly solved, or the time step size varied dynamically ( explanation https://en.wikipedia.org/wiki/Explicit_and_implicit_methods). I think this was the most asked question by people who were running UWG and usually because they have specified a very thin (membrane) or high-conductance layer (metal) which would change temperature much more rapidly compared to everything else.

Joseph

On 14 June 2018 at 12:15, Saeran Vasanthakumar notifications@github.com wrote:

@hansukyang https://github.com/hansukyang thanks, that's exactly what I was looking for!

I just edited the code here: https://github.com/ladybug- tools/urbanWeatherGen/blob/ea6695dbcaa3f188a12a67b94ff067 ade5c087ac/UWG/building.py#L179-L199

I'm also not sure if a NaN (or something else) will be generated in python in this case, so I added a try/except loop, for the temperature bound checking.

We'll still have to figure out how to solve this error, but at least now I'm confident we'll catch it when it occurs.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ladybug-tools/urbanWeatherGen/issues/3#issuecomment-397165211, or mute the thread https://github.com/notifications/unsubscribe-auth/ALUG7UZYCTr93i7Pfm4fhK18Pk5Jbmqnks5t8eNNgaJpZM4NFIGq .

saeranv commented 6 years ago

Thanks @hansukyang,

I was talking to @chriswmackey today, and we have both observed that this python version of UWG, based on the .m inputs has yet to reproduce the FATAL ERROR bug. Obviously, this could just be because the error-checking hasn't caught it thus far, but even after I've implemented it, none of my tests were catching any mistake, even though I was running into this error almost every time I ran an epw file in the old Matlab code. I myself suspect that the .xml file input method may have been contributing to the problem.

So for now, I think we are going to do an initial release of the the UWG (through Dragonfly) and see if any of the Ladybug users encounter a bug. That way we can reassess how prevalent it is in this version, and we can discuss getting some help from you, or someone with the appropriate background to rewrite the heat conduction calculation.

S

hansukyang commented 6 years ago

The .m and .xlm input files are treated somewhat differently (due to request to leave the .xlm unchanged, from Les' colleagues at MIT who were using UWG) as I remember. On the .m and Excel input, thin materials such as metal decking and membranes are essentially not considered as their addition to the thermal resistance is negligible (btw, this is a commonly used approach in thermal model simulation, where thin materials are considered only for their albedo/emissivity value as 'skin' with zero thermal mass) so yes, it makes sense that these are not encountered (I don't think .m files modified the building roof/wall etc. types). If Ladybug users are only using the .m input, then I think you should be good to go.

Joseph

On 19 June 2018 at 10:31, Saeran Vasanthakumar notifications@github.com wrote:

Thanks @hansukyang https://github.com/hansukyang,

I was talking to @chriswmackey https://github.com/chriswmackey today, and we have both observed that this python version of UWG, based on the .m inputs has yet to reproduce the FATAL ERROR bug. Obviously, this could just be because the error-checking hasn't caught it thus far, but even after I've implemented it, none of my tests were catching any mistake, even though I was running into this error almost every time I ran an epw file in the old Matlab code. I myself suspect that the .xml file input method may have been contributing to the problem.

So for now, I think we are going to do an initial release of the the UWG (through Dragonfly) and see if any of the Ladybug users encounter a bug. That way we can reassess how prevalent it is in this version, and we can discuss getting some help from you, or someone with the appropriate background to rewrite the heat conduction calculation.

S

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ladybug-tools/urbanWeatherGen/issues/3#issuecomment-398254996, or mute the thread https://github.com/notifications/unsubscribe-auth/ALUG7RMWKn0c494MRgiqZjhq_wQ59LIRks5t-GJ8gaJpZM4NFIGq .

saeranv commented 6 years ago

@hansukyang, yes I noticed that too when I was looking at the .xml method in the Matlab code. And the .m files don't modify any of the BEMDef materials so I agree this should be more stable.

I'll leave this issue up for now, so that we can revisit when/if the issue comes up again.

hansukyang commented 6 years ago

Sounds good.

On Wed, Jun 20, 2018, 11:37 AM Saeran Vasanthakumar < notifications@github.com> wrote:

@hansukyang https://github.com/hansukyang, yes I noticed that too when I was looking at the .xml method in the Matlab code. And the .m files don't modify any of the BEMDef materials so I agree this should be more stable.

I'll leave this issue up for now, so that we can revisit when/if the issue comes up again.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ladybug-tools/urbanWeatherGen/issues/3#issuecomment-398614234, or mute the thread https://github.com/notifications/unsubscribe-auth/ALUG7crKLZqJIIEGTz4n9KMsXEUdmKbAks5t-cOSgaJpZM4NFIGq .

saeranv commented 6 years ago

@hansukyang @chriswmackey

Heads up, the FATAL ERROR due to extremely high calculated interior ceiling temperatures has come up in Dragonfly here: https://discourse.ladybug.tools/t/error-running-uwg-caused-by-weather-file/3400

It's using the .m method of inputting the construction assemblies (via the DOE building typologies), so that method may not have prevented the numerical error as we initially thought.

So, perhaps we do have to change the heat balance formula to solve this...

S

hansukyang commented 6 years ago

Yikes - it's unfortunate that this is coming up. Since the new Python version is much faster in iteration, one quick workaround may be to decrease the time-step. Out of curiosity, if you have the files to reproduce the error, try reducing the time-step until the error goes away and see if it's a reasonable quick fix?

On 4 September 2018 at 02:14, Saeran Vasanthakumar <notifications@github.com

wrote:

@hansukyang https://github.com/hansukyang @chriswmackey https://github.com/chriswmackey

Heads up, the FATAL ERROR due to extremely high calculated interior ceiling temperatures has come up in Dragonfly here: https://discourse.ladybug.tools/t/error-running-uwg- caused-by-weather-file/3400

It's using the .m method of inputting the construction assemblies (via the DOE building typologies), so that method may not have prevented the numerical error as we initially thought.

So, perhaps we do have to change the heat balance formula to solve this...

S

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ladybug-tools/uwg/issues/3#issuecomment-418172105, or mute the thread https://github.com/notifications/unsubscribe-auth/ALUG7Qg80VaL4-yQOTfleUIBXxtSIiTtks5uXXGDgaJpZM4NFIGq .

saeranv commented 5 years ago

@hansukyang

Sorry for taking a long time to get back to you!

Quick update: I just did a quick test and reducing it to 150 solved the problem for the timestep where it was occurring. I tried to push it to 250, which threw another bug which I think is unrelated and that I'll have to hunt down.

Will keep you updated.

saeranv commented 5 years ago

@hansukyang

Sorry for the late update here, but can report that reducing the timestep worked, and there aren't any other contributing factors in the Grasshopper tool that is contributing to the problem (that I can see).

I had to make a number of edits to the UWG library, and found some bugs in the Grasshopper tool, while trying to reproduce the issue outside of Grasshopper, in the core UWG package, (see here https://github.com/ladybug-tools/uwg/commits?author=saeranv, and here: https://github.com/chriswmackey/Dragonfly/issues) but long story short, after fixing all the little bugs that were in the tool the problem still existed, and therefore reducing the timestep is the only way to fix this problem.

So I've changed the defaults timestep to 200s, and have revised the error message to advise users to try reducing the timestep if they continue to run into the error.

Thanks again,

-S

hansukyang commented 5 years ago

Great to know it works at least and thanks for the update Saeran.

Joseph

On Sun, 25 Nov 2018 at 03:28, Saeran Vasanthakumar notifications@github.com wrote:

@hansukyang https://github.com/hansukyang

Sorry for the late update here, but can report that reducing the timestep worked, and there aren't any other contributing factors in the Grasshopper tool that is contributing to the problem (that I can see).

I had to make a number of edits to the UWG library, and found some bugs in the Grasshopper tool, while trying to reproduce the issue outside of Grasshopper, in the core UWG package, (see here https://github.com/ladybug-tools/uwg/commits?author=saeranv, and here: https://github.com/chriswmackey/Dragonfly/issues) but long story short, after fixing all the little bugs that were in the tool the problem still existed, and therefore reducing the timestep is the only way to fix this problem.

So I've changed the defaults timestep to 200s, and have revised the error message to advise users to try reducing the timestep if they continue to run into the error.

Thanks again,

-S

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ladybug-tools/uwg/issues/3#issuecomment-441390285, or mute the thread https://github.com/notifications/unsubscribe-auth/ALUG7UMOlAP_DgYVWvNGlXb5zB77r8qEks5uyZ36gaJpZM4NFIGq .