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

Revise time-step calculation in UWG #3

Open
saeranv opened this issue Apr 22, 2017 · 16 comments
Open

Revise time-step calculation in UWG #3

saeranv opened this issue Apr 22, 2017 · 16 comments

Comments

@saeranv
Copy link
Member

saeranv commented Apr 22, 2017

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
Copy link
Member

@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
Copy link
Member Author

saeranv commented May 27, 2018

@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
Copy link

hansukyang commented May 28, 2018 via email

saeranv added a commit that referenced this issue May 29, 2018
@saeranv
Copy link
Member Author

saeranv commented Jun 10, 2018

@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
Copy link

hansukyang commented Jun 11, 2018 via email

@saeranv
Copy link
Member Author

saeranv commented Jun 14, 2018

@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
Copy link

hansukyang commented Jun 14, 2018 via email

@saeranv
Copy link
Member Author

saeranv commented Jun 19, 2018

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
Copy link

hansukyang commented Jun 19, 2018 via email

@saeranv
Copy link
Member Author

saeranv commented Jun 20, 2018

@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
Copy link

hansukyang commented Jun 20, 2018 via email

@saeranv saeranv removed the critical label Jun 24, 2018
@saeranv
Copy link
Member Author

saeranv commented Sep 3, 2018

@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
Copy link

hansukyang commented Sep 6, 2018 via email

@saeranv
Copy link
Member Author

saeranv commented Oct 2, 2018

@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
Copy link
Member Author

saeranv commented Nov 24, 2018

@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
Copy link

hansukyang commented Nov 26, 2018 via email

saeranv added a commit that referenced this issue Mar 11, 2020
chore(uwg): Pull version update.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants