search

pollination / sky-irradiance

Pollination recipe to calculate annual radiation based on Tregenza/Reinhart sky subdevisions
Other
0 stars 4 forks source link

Can we have the recipe accept a .sky as input? #8

Open chriswmackey opened 3 years ago

chriswmackey commented 3 years ago

Looking over the high potential for the inputs to conflict with one another (eg. a Wea that's for a leap year without leap year set to true), this recipes just seems like it would be a lot more reusable and elegant if it started from a sky instead of a Wea.

There also seem to be some contradictions between the name of the recipe and the inputs. For example, radiation is always cumulative (in Wh/m2) so the cumulative input seems like a misnomer. I guess if the cumulative input is hourly the output is irradiance (in W/m2) and not radiation? Or maybe the recipe is mis-named? Should just be called sky-irradiance?

I also understand if this recipe is just for a special case and we want to leave it be and add a different recipe for cumulative radiation. But I just wanted to say that this recipe doesn't really look like one that I would want to expose on the LBT Grasshopper plugin in its current state.

mostaphaRoudsari commented 3 years ago

Hey @chriswmackey, Thank you for opening this issue. I can indeed use some help to improve this recipe.

this recipes just seems like it would be a lot more reusable and elegant if it started from a sky instead of a Wea

To get parse_sun_up_hours to work I need to know if it is a leap_year or not. We can add an extra step to check the wea file and see if the date for 2,29 is used but this is not something that we would know from a sky file.

There also seem to be some contradictions between the name of the recipe and the inputs. For example, radiation is always cumulative (in Wh/m2) so the cumulative input seems like a misnomer. I guess if the cumulative input is hourly the output is irradiance (in W/m2) and not radiation? Or maybe the recipe is mis-named? Should just be called sky-irradiance?

I don't follow all your comments here but I agree that the choice of words are not the best. So here is why the names are the way there are. I don't like the way they are but that's the best that I could come up with. If you have better suggestions I think we should change them.

Cumulative in this recipe can be used to generated a cumulative sky materix (e.g. gendaymtx -A). Otherwise the results will be generated for every hour.

I called it annual-sky-radiation since this recipe generates the radiation from the sky patches and not the sun disk. It's really not a good choice but I couldn't come up with a better one. What would you call it?

I also understand if this recipe is just for a special case and we want to leave it be and add a different recipe for cumulative radiation.

It's not meant to be. This should be the radiance-based alternative to Ladybug's Incident Radiation component. It's more flexible and does provide hourly calculation too but I think people would mostly use it for cumulative incident radiation.

chriswmackey commented 3 years ago

Hey @mostaphaRoudsari,

I just wanted to let you know that I read your response, I have been thinking about this, and I have a number of suggestions that might improve the recipe.

I understand now why the timestep is needed (because you need to know the interval of time represented by each step of the Wea in order to convert Radiance's averaged irradiance output into cumulative radiation). However, we should be able to get away with not having to specify whether it's a leap_year or not since this information (if relevant) is already included in the Wea. I'll investigate this further but I'm pretty sure we'll be able to remove the leap_year input before pushing the recipe to production.

Also, now that I read your explanation, I think that I might understand the purpose of this "hourly" input and that it is just a misnomer. If you really wanted to include it, the name of the input would be much more clearer if you took Radiance's gendaymtx suggestion and called it averaged. Then, the input string could be either "cumulative" or "averaged", which are much clearer opposites from one another than "cumulative" vs. "hourly". To explain my original confusion, I kept thinking that, if someone wants an hourly irradiance value, they can use the point-in-time recipe. But now I see that this recipe is meant to be an average irradiance value across all of the time intervals of the Wea.

While we could change the input to averaged, I think that I have a better suggestion. Instead of having this averaged input, we could have two outputs from the recipe (one for the averaged irradiance and another where this averaged number has been multiplied by the total time interval of the Wea to give you cumulative radiation). Then, I think we could safely change the name of the recipe to "Cumulative Radiation", which is a much clearer description of the METRIC/OUTPUT the recipe is trying to produce rather than HOW THE CALCULATION IS RUN (like it is now). If you ask me, this is the recipe that you use when you only care about radiation over long periods of time (eg. cumulative radiation) and you only need to use the other options of "annual radiation" and "point-in-time grid-based" recipes when you care about the accuracy of irradiance at individual points of time.

What do you think?