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

Simplified radiation script #3287

Merged
merged 8 commits into from
Jun 28, 2023
Merged

Simplified radiation script #3287

merged 8 commits into from
Jun 28, 2023

Conversation

reyery
Copy link
Member

@reyery reyery commented Feb 11, 2023

Created a simplified script for radiation that runs radiation on a subset of buildings in the area and tries to get sample data for each building surface (i.e. average W/m2 for each surface roof, wall, window).

cea radiation-simplified --sample-buildings B1005

Based on the reference case, choosing the building in the middle as the sample, roofs are quite accurate but the other surfaces are around 50% higher in total. The script also takes half the time needed.

Error could be improved by using more sample buildings, but i have not tested it.

Percentage diff of total
roofs_top_kW, 1.238405
walls_east_kW, 23.794536
walls_north_kW, 49.347508
walls_south_kW, 55.284678
walls_west_kW, 72.813562
windows_east_kW, 26.135608
windows_north_kW, 48.931709
windows_south_kW, 53.819722
windows_west_kW, 68.489829

@reyery
Copy link
Member Author

reyery commented Feb 11, 2023

Let me know of any ideas that could help

@reyery
Copy link
Member Author

reyery commented Feb 14, 2023

Added a buffer parameter to the script, which defaults to 50m, which takes into account buildings within the buffer of the sample buildings, for shading.

Error seems to improve by a bit.

roofs_top_kW,1.371222
walls_east_kW,4.058351
walls_north_kW,-31.369054
walls_south_kW,-19.301223
walls_west_kW,49.176562
windows_east_kW,11.542085
windows_north_kW,-24.050071
windows_south_kW,-32.029846
windows_west_kW,46.898999

@jimenofonseca
Copy link
Contributor

@reyery so one check did not pass, do you know what his could be?

@jimenofonseca
Copy link
Contributor

@lguilhermers are we ready to merge?

@ShiZhongming
Copy link
Collaborator

I will test this one before we proceed to merge

@ShiZhongming ShiZhongming self-requested a review April 27, 2023 09:28
@ShiZhongming
Copy link
Collaborator

ShiZhongming commented May 1, 2023

@reyery
I have just tested this PR on a cluster of 10 typical HDB buildings in Clementi, Singapore.
The time used for the radiation command was: 493 seconds, compared to the original radiation command: 776 seconds.
In addition to radiation, I have also executed the photovoltaic.

Accuracy-wise, the total PV electricity yield (roof+facade) ranges from 2%-8% difference from that using the original script. The roof PV electricity yield is almost the same to that using the original script. Like you said, the main difference is from facade.

I think this PR may be a valuable asset when running a really large amount of buildings at once. Hence, I propose to merge this PR for the next release with a good documentation introducing the pros and cons using this script.

@jimenofonseca
Copy link
Contributor

jimenofonseca commented May 1, 2023 via email

@ShiZhongming
Copy link
Collaborator

@jimenofonseca
That's good idea - having the fast track as a selection when executing solar radiation.
Currently, this new fast track command is run separately.
What do you think? @reyery

@lguilhermers
Copy link
Collaborator

Hi everyone,

Sorry, I was not able to review this branch and believe I won't be able to give a very detailed look into it as well (for time and resource constrains we decided to use another approach to estimate city scale energy demand for our scenarios).

Nonetheless, I think it is a good addition and should be integrated into CEA.
I would just be careful when making it explicit to the used (with a flag as mentioned by Jimeno), since it generates a lot of simplification and hasn't been tested into a wide range of scenarios (e.g., heterogeneous building heights and properties, and different countries). To avoid being misused and generating misleading results, I would at least add documentations and warnings that alert to that.

@reyery
Copy link
Member Author

reyery commented May 2, 2023

Thanks for all your comments. I do feel that the script should still be standalone i.e. radiation-simplified instead of a flag in the main radiation script, mostly because doing that would also add a few unrelated config parameters to the main radiation script. And running it should be more explicit, because the user might forget to check the flag.

But I am open to change it to a flag if you all think it is a better idea.

@jimenofonseca
Copy link
Contributor

jimenofonseca commented May 2, 2023 via email

@reyery
Copy link
Member Author

reyery commented May 2, 2023

Maybe not a flag but a drop down with two options. Simplified and standard. Standard shall be the default. In the description it should say explicitly:“ the results of the simplified are up to 30% diferent but 30% faster Than the standard calculation (use it at your own risk)“Make two sections in the config: one for simplified and one for the standard.

Alright, I shall do it according to this.

@MatNif
Copy link
Collaborator

MatNif commented May 3, 2023

Hi everyone,

Sorry to pitch in on this so late.

The new code of the script makes perfect sense in my opinion. The only suggestion I would have, is to create a DocString for all the main methods summarising their purpose.

I tested the script on a zone with close to 400 buildings and selected roughly 15 of them as a proxy for the radiation calculation. As expected, the daysim portion of the script runs a lot faster than of the main radiation script. However, I noticed that the geometry generation and the sensor placement still take a very long time to run.

As I looked into it a bit further I discovered that the 'roof-gird' and 'wall-grid' parameters under the 'Level-of-Detail' section of the config do absolutely nothing. My 'wall-grid' parameter was set to 200, which is meant to create only 1 sensor per wall surface according to the parameter description. However, for a building that I picked at random, more than 250 sensors were placed on each wall.

I believe if that is fixed we could reduce the computation time of the radiation script by a lot more.

This is not meant to take anything away from this script. I think it makes absolute sense. But if anyone finds the time to continue working on increasing the speed of this script, I think this would be a point to take up.

@ShiZhongming
Copy link
Collaborator

Thank you @MatNif for taking a deeper look.

When you said "wall", did you mean the entire facade should be having 1 sensor using 'wall-grid' parameter at 200? The way the facade surfaces are (i.e. each orientation to be split by foor by window/wall, by wall above/between/below window) is making 200 not possible I think. My understanding is that: it only makes senses when setting a much smaller number, hence reaching higher level of details. Hence, setting the wall-grid at 200 will never do anything or help reducing the computing time.

Correct me if I am wrong? : )

@MatNif
Copy link
Collaborator

MatNif commented May 3, 2023

Hi @ShiZhongming,

You might be right, I might have misunderstood.
The default.config (and thus the parameter description in the dashboard) states the following:
"Grid resolution for wall surfaces. Use 200 (maximum) if you want only one point per surface."

I assumed that meant only one sensor-point would be generated on each wall (i.e. one on the west-facing wall, one on the east-facing wall etc. for a typical building with a rectangular footprint). But you might be right, the intention behind it might have been that one sensor-point is create per wall segment, i.e. one per floor and per space between two windows.

However, unless I have missed something, there is no reference to either the 'wall-grid' or 'roof-grid' parameter in the radiation script, nor its subroutines. The parameter just doesn't get called and therefore has no influence on the script.

Copy link
Collaborator

@MatNif MatNif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code seems to work as intended. No issues with the changes.

@ShiZhongming
Copy link
Collaborator

Maybe not a flag but a drop down with two options. Simplified and standard. Standard shall be the default. In the description it should say explicitly:“ the results of the simplified are up to 30% diferent but 30% faster Than the standard calculation (use it at your own risk)“Make two sections in the config: one for simplified and one for the standard.

Alright, I shall do it according to this.

@reyery shall we merge this PR to include this fast solution asap? and the user-defined possibility to use this fast solution in another PR?

@reyery reyery merged commit 8ce5db8 into master Jun 28, 2023
3 checks passed
@reyery reyery deleted the simplified_radiation branch June 28, 2023 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants