ladybug-tools / ladybug-legacy

:beetle: Ladybug is an environmental plugin for Grasshopper.
http://ladybug.tools
Other
194 stars 82 forks source link

Suggestions for improving set of Photovoltaics components #150

Closed stgeorges closed 6 years ago

stgeorges commented 9 years ago

Hi @chriswmackey , @mostaphaRoudsari , @antonszilasi

As Mostapha suggested, I am continuing an email conversation in here, related with suggestions on improving Photovoltaics components.

Hi Chris,

Thank you for the reply and all the thorough comments and suggestions.

I can not commit new files to github, as I can not install github application. It requires Windows XP >, and on my PC I have Windows XP. I tried using other github client (older version of smartgit) but without success. Mostapha suggested using Tortoise git, I see if I could wrap my head around its way of working in the next period. So I have no problem with you (or Mostapha) committing the files.

1) I would strongly suggest just condensing all of these epw inputs into a single "_epwFile" input and you just parse the epw file inside the photovoltaics surface component

Yes, your recommendation definitively has a point. As you remember I had _epwFile input for the "Wind boundary profile" component last year. Then I realized that it is much quicker (in terms of component solution speed) to have separate weather inputs than unpacking the epw file weather data inside the component. On my PC "unpacking" epw file within the component added 2.6 seconds to the overall component solution time. If I remember correctly your PC was a couple times faster than mine. I know you mentioned that it was a couple of years old. Still I imagine that this is according to USA standards. Out of USA, there may be more people with PC speeds like mine. Nevertheless I replaced all weather inputs with single epwFile one, as you recommended. Maybe we won't get any speed complaints, let's see how it goes.

Along the lines of minimizing inputs, I question whether anyone is going to want to input a numeric tilt angle and azimuth angle

These two inputs are for advanced users. Instead of surface, (or numeric value input of surface area), one can input system size/nameplate rating of a PV array/module instead. This is in fact how PVWatts works, it does not take surface area as an input. When system size/nameplate DC rating is inputted, tilt and azimuth angles have to be defined manually.

2) A lot of your outputs on the "photovoltaics surface" component are just taking the same data and expressing it in different ways and we already have Ladybug components to help users parse through data as they wish so these outputs are a bit redundant. I understand if you want to provide both an hourly and an annual output for the AC energy as I know this energy is the most important output and users might want a quick-access total of the energy.

Radiation values are also part of the AC energy analysis. To keep the results consisted, I have to use the Michalsky and Perez position/solar radiation results, not the ones implemented by other components. Still I will listen to you. I would still keep the "totalRadiationPerHour" as it is required for "Photovoltaics performance metrics" component.

However, users can get the monthly values by using the other LB components and so I do not think it is necessary to have them as outputs.

I would say it is far better to have a single output calculating monthly values than having to use the Ladybug's "Average Data" component along with "Analysis Period" component just to extract a single monthly value. Monthly values are a significant part of AC energy analysis.

Also, if a user really wants the total or average daily radiation for the year, they can use the native GH mass addition or the Ladybug Average Data component so these outputs are really redundant.

I would comment the same on this, as with upper suggestion: It is much more convenient to have them as an output than requiring the user to use the upper mentioned two Ladybug components.

Furthermore, the surface area of the PV is really not too helpful as users can use the native GH area component to calculate this.

This is actually a result of multiplication of "PVsurface" with "PVsurfacePercent" input parameters, not the actual area of the "PVsurface" alone. I fixed the description and the actual name of this input, thank you for noticing the mistake.

Finally, I don't think that you need quite so many dividers for these outputs once you narrow down the number of outputs.

This is a personal preference, to divide different types of output parameters, but then again I removed 2 out of 4 of them, due to your suggestion and removal of some of the mentioned previous outputs. Thank you.

3) You might want to put in a check on the "Sunpath Shading" component to make sure the component does not run with null values connected for geometry (see attached GH file).

Fixed it. Thank you.

4) I am guessing that the black in the "SunpathShading" component refers to areas of the sky blocked by the context geometry but it would be great to see exactly what this means in the legend (does this mean the annual AC percentage is 0?)

Check the "sunWindowMesh" output explanation. Black areas represent the 100% shaded portions of the sun(solar) window. It does not matter whether or not that particular part of the sky dome "generates" AC energy or not (or solar radiation if you are using the component for other non-Photovoltaics purposes). I added three new blocks to the legend: Shading from "context", "coniferous trees", "deciduous trees". Inserting transmission indices would require differentiating between deciduous inleaff and leafless trees, which is something that would cause confusion in the sunpath diagram. I would rather not add these, and keep the present edited state.

5) The way that you have setup the workflow in the example file causes a recursive loop, which Grasshopper intrinsically hates (see the second GH file).

You skipped the steps 4, 5 in the old "B_shaded_analysis_PhotovoltaicsSurface.gh" file.

Is there any reason why we could not just get an adjusted ACenergyPerHour output from this component instead of having to plug it back into the old workflow?

No, the annual shading value has to be returned to the losses component.

Reducing all annual shading to a single percentage seems to be an oversimplified way of accounting for shading effects anyway.

That is the way PVWatts works, it accounts for all the losses (age, wiring, soiling, shading...) in a single loss parameter. A more precise shading data would be using "beamShadingPerHour" (I changed its name to "beamIndexPerHour" as it is in fact transmission index).

"Sunpath shading" basically uses two methods: one hourly - to analyse for sun position blocking, and all the rest ones (annual, autumn-winter, spring-summer) by using the Sunpath diagram method described in the mentioned paper, in components description. Sunpath diagram method described in the paper, essentially tries to replication what Solar pathfinder tool and application does - it takes a snaphot of the sky dome, and checks for the obstructions within a sun(solar) window, based on blocking of particular sun window's active quadrants. I am not an engineer, nor this was my field of study. Reading a book or two, and a couple of papers does not make one an expert in this field. Maybe Anton could explain more on the shading issue in detail. In general shading of Photovoltaics is a complicated area. It depends on a lot of factors. It depends on the time and position of shadows from nearby context objects traveling across PV array. Cloud cover, air pollution and haze. The way modules or arrays are connected. Whether or not PV panels have by-pass diodes, micro-inverters, power-optimizers. Neither PVwatts, PhotovoltaicsSurface component (based on PVWatts), Solar pathfinder, nor SunEye address this issue. I doubt we will ever have something this sophisticated for Ladybug+Honeybee.

So the final conclusion is that "beamIndexPerHour" method underestimates the shading. It can be used though as more precise when one considers a PV array to be "unshaded". The annual version on the other hand is a bit conservative meaning it yields a bit larger shading results. In general it can overestimate the shading of "unshaded" PV arrays. But in real life world, unless you are constructing a PV array in the middle of Salar de Uyuni, you will always have some percentage of annual shading, ranging from 3-5%. That is why we decided to stick with the conservative method for Photovoltaics. And to use an hourly ("beamIndexPerHour") for Solar hot water collectors and shading analysis of any other Ladybug+Honeybee components.

BTW, I noticed that I get a null output for the beamShadingPerHour when I plug in the ACenergy. Is this supposed to happen? 6) I also noticed that I get null outputs for most of the other shading outputs on the "SunpathShading" component when I do not plug in an ACenergyPerHour. Maybe a tutorial video or something could clarify how I am supposed to use the components because I feel like I am doing it wrong now or at least not understanding why I only get certain outputs in certain cases.

Yes it is suppose to happen. At least I thought so. I tried to differentiate between shading data for Photvoltaics and for Solar hot water and other purposes. Now that you have mentioned maybe this is a bad idea. I included "You need to input ACenergyPerHour" to all outputs which require this data to be inputted, in order for them to be calculated. Yes of course, video tutorials are going to follow, once the components are released, and I get some spare time.

7) Again, it seems like you have a lot of redundant outputs on the "SunpathShading" component. Users can calculate a Sep21toMar21Shading using the native GH components if they want.

You can not get it by using native grasshopper components, due to differences in methods used to generate the shading values.

They can also get the WindowCentPt with the native GH components.

"sunWindowCenPt" output resembles the "sunPathCenPts" output of the Ladybug "Sunpath" component. I do not see an issue with this principle, of having a center point for the overall geometry as an output.

Finally, I don't think that we are gaining all that much form the hours and hour positions here that users could not get by using the Ladybug Sunpath component.

Are you suggesting using Ladybug "Sunpath" component along with "Sunpath shading" just to mark the solar time hours of "Sunpath shading" sunpath diagram? That sounds a bit exaggerated, at least to me. I do not see a problem in marking solar hour positions, they are actually beneficial.

8) The analysisPts and quadrant centroids are useful for understanding how the component runs but I imagine that users do not really want to see it all of the time. I would hide them as an output such that users only see them if they connect them to a grasshopper Point component.

The whole "Sunpath shading" component is hidden ("Preview" turned off) by default, so neither of these two are seen. I personally think this is a better, universal solution than hiding particular outputs.

9) The sunWindowShadedArea output is really misleading because it seems to be a percent - not an actual area in rhino model units.

You are right. I will change its name to "sunWindowShadedAreaPer". Thanks.

10) as recommended previously, take out the hourPositionsScale.

I do not see a need for its removal. Increase the "scale" input parameter of the "Sunpath shading" component and you will see why having "hoursPositionScale" input is important.

Here are the newest revised example files according to suggestions: http://www.megafileupload.com/8ODX/Ladbybug_Photovoltaics_exampleFiles.zip Mostapha, I send you the .ghuser and .py files to your email few minutes ago.

One more time, thank you for the advices and suggestions Chris. They have really been helpful in making these components far better and useful.

mostaphaRoudsari commented 9 years ago

Hi @stgeorges,

Thanks for starting this issue. Let' break down this issue in multiple issues as we go on. It's really hard to keep track of 10 items in a single issue.

I committed the new versions of the changes (b36fe6ccef240b4cb1ec3f9af7165fbe13c85ab7).

Two question/suggestion about the sample files.

  1. Is there any reason that you don't internalize the geometries in Grasshopper files? In that way we don't need to upload the Rhino files. It also makes it much easier for users to get it set-up.
  2. What do you think about using "Ladybug_Open EPW And STAT Weather Files" component to get the weather file so we don't have to send the weather file too.

If we do these two then we can only share grasshopper files which will make everybody's life much easier!

stgeorges commented 9 years ago

Hi @mostaphaRoudsari ,

Thank you for the commits!

  1. I replied to this question two weeks ago in an email, maybe you missed that part. Grasshopper (0.9.0076, 0.9.0075, possibly earlier releases too) has an issue with internalizing breps. Once you download a .gh file with an internalized brep and save it, the next time you open it, internalized brep is gone (gets "Null"). This is not always true, sometimes brep is internalized for good, without problems. The possible workaround is to convert breps to meshes, which I do not think is a good idea (meshes can always be internalized without problems). The purpose of the example file is to show how different geometries (breps, surfaces, meshes) can be used as context objects. If you think that this is irrelevant and that using .gh files only instead of additional .3dm ones, is far more important, I will convert breps to meshes.
  2. I use the "file path" parameter with shipped .epw file, as an opportunity for those users who do not have access to internet (yes, you need an access in order to download the very example files, but still this can be done at the university, in an office...). Maybe this may sound a bit strange to you guys, but not everyone has an access to the web the way people have in the Western world. I myself have been using 56kb connection less than 4 years ago, and it was expensive as hell. So the middle ground would maybe be adding "Ladybug_Open EPW And STAT Weather Files" component to the definition, and shipping the .epw file too?
mostaphaRoudsari commented 9 years ago

Hi @stgeorges

  1. It is ok. We have never had this issue so far. Let's internalize the breps and in case anyone reports the issue we'll share the Rhino file with them. [Let's focus on the potentials!]
  2. I hear you. Let's just keep it as it is. Thanks!
stgeorges commented 9 years ago

@mostaphaRoudsari

  1. I should have googled first. I apologize. Looks like it was Rhino 5 SR <10 that was causing the issue. Versions 10 >= do not show this behavior. Still for us with older versions (I have SR 9 and SR5) wouldn't it better to skip all the hassle about reporting, and just ship both .gh and .3dm files altogether, but having breps internalized in .gh files?
mostaphaRoudsari commented 9 years ago

@stgeorges I'm cool with that. Let's internalize the breps and also have Rhino files there just in case.

stgeorges commented 9 years ago

Thank you.

chriswmackey commented 8 years ago

@stgeorges and @mostaphaRoudsari ,

I am sorry for starting a duplicate issue of this one and I will post my response to this duplicate issue here (https://github.com/mostaphaRoudsari/ladybug/issues/188) . Somehow I had forgotten that I had already given a lot of this feedback. I am happy to see that many of these suggestions have been followed (like the helpful epwFile) and I buy many of your arguments.

However, there is one that I feel you really need to implement and that I belive @mostaphaRoudsari would support me in implementing.

2) I understand your argument for having hourly data in many different formats output from your component but this really creates a nightmare for people like @mostaphaRoudsari and I when we fix bugs, update code, and (in the future) port the code to other platforms (like Dynamo). We have made one solid and good component to separate hourly annual data into monthly and daily values (Ladybug_Average Data) so that we don't end up with 50 components each with their own unique function for separating annual data or components with 50 outputs with each annual data stream in a different format. Having you put the outputs in all of these formats on your component is generally going to make it harder for us to do things like porting to Dynamo in the future if all of the developers do this. It also defeats some of the purpose of working in a visual scripting interface that allows you to customize a script to focus on the data that you are interested in. For example, this is why Honeybee is so much faster than other interfaces for EnergyPlus - If you don't want data in a certain format or to use a certain feature of E+, you just don't drop down that component on the canvas and it never ends up increasing the time of your script. You are going against this philosophy when you forcibly make the user accept all of the formats of the output as opposed to giving them the option of dropping the "Average Data" component on the canvas or not.

I buy the argument that you want to accommodate an engineer's perspective on how they would design a PV array by allowing people to simply input numbers and not geometry (I imagine that there is a small user base that does not understand the geometry of Rhino well but wants to use the environmental analysis tools). However, I would also argue that, if you are simply going to make PVWatts for Ladybug, why not just use the original PVWatts? Your code should be adding in some capability that you do not get with the online tool and, for me, this primary value that you are adding is the ability to visualize what you are simulating geometrically in the Rhino interface (something that you currently do very well with the shade factor component, for example). As such, I would be much happier if we just made another component that generates a PVSurface for you to run through your component based on these numbers rather than having some black-box calculation run inside your component where the user potentially does not understand what they are simulating. So I would suggest something either like this: pvcomponent suggestion Or you have the component blank out the tilt and azimuth inputs if a PV surface is connected (so that i's clear to users that these inputs are not being accounted for) and you output the geometry of the panel in the case of a tilt angle and azimuth being used. Right now, the situation is dangerous for users to not know what they are simulating, though, and this has to be changed in some way or else we could be dealing with claims like "Ladybug is not accurate."

-Chris

mostaphaRoudsari commented 8 years ago

@stgeorges I agree with @chriswmackey's points in his last comment. What are your thoughts on this? Do you also find them reasonable?

stgeorges commented 8 years ago

Hi @chriswmackey , @mostaphaRoudsari ,

2) Some monthly outputs can be removed, some can not. I already removed the ones that can be calculated from hourly values by using "Average Data" component. Thank you. Monthly outputs which can not be calculated from hourly data, are cumulative monthly outputs. Here is an example:

cumulativemonthlyvalues

So each TaCorrectedPR is calculated based on cumulative values of PacPerMonth, TcellPerMonth, EpoaPerMonth (summation of: AC energy per month, cell temperature per month, solar radiation per month, respectively).

2) b) I think having these inputs can be beneficial in some cases, and after all, if one hovers a mouse pointer over them, there are descriptions which explain how they work. But I think you are right, overall. I will remove them, and make a new component which has a tilt and azimuth angles as inputs. _PVsurfaceand PVsurfacePercent_ will only remain.

My apologies, and thank you once again for useful suggestions!

chriswmackey commented 8 years ago

@stgeorges , Thanks for the explanation and it makes sense. I did not realize that the results you were putting in this monthly format were cumulative. It has occurred to me that this cumulative total should really be an option on the "Average Data" component and I have added an issue for it here: https://github.com/mostaphaRoudsari/ladybug/issues/210 I guess the average data component was not as good as I thought it was and thank you for pointing out this limitation. In the meantime, your intended edits to the components sound good. -Chris

stgeorges commented 8 years ago

@chriswmackey , Looks like my lack of knowledge of English language finally got me. I was not suppose to use the word "cumulative". The correct one should be: "summation". "Average component" already has this option: Total.

But the upper temperatureCorrectedPRperMonth for each month is calculated by summation of various data for each month, and then performing operations on those summations. Here is sort of grasshopper example of how it would look like with "Average Data" component (PacPerMonth, EpoaPerMonth, TcellPerMonth first need to cull all values for which EpoaPerHour is less or equal than 0.6, but I will skip that step):

tempcorrpr http://www.filedropper.com/tempcorrpr_1

This is why it is impossible to calculate it by using an hourly data and "Average Data" component. I hope it makes it more sense now.

My apologies for using the wrong terminology.

chriswmackey commented 8 years ago

@stgeorges ,

Different terminology that leads to the realization of useful features should never be apologized for. I understand that implementing a vumulative option will not fix your issue but I think that I will implement it anyway as I see some cases where it can be useful.

Also, great use of the medium of visual scripting to make clear the calculation that you are performing. Because of the way that you have depicted it, I instantly understand that it is an operation that needs its own output.

-Chris

chriswmackey commented 6 years ago

Now that this issue has outlived its purpose and the renewables components are a stable part of the project, I am closing out this issue.