ladybug-tools / honeybee-legacy

:bee: Honeybee is a free and open source plugin to connect Grasshopper3D to EnergyPlus, Radiance, Daysim and OpenStudio for building energy and daylighting simulation
http://ladybug.tools
Other
125 stars 145 forks source link

Where to Put Electric Lighting Components Once Out of WIP #540

Closed chriswmackey closed 8 years ago

chriswmackey commented 8 years ago

@sariths ,

I have been testing out your electric lighting components today and have been working well. I agree that they are ready to come out of WIP. I just wanted to know if you had any thoughts on where they would best fit in the current Honeybee tabs. I could see them fitting under the "02 | Daylight | Sky" tab (maybe we change it to "02 | Daylight | Light Sources").

I could also see them fitting under "04 | Daylight | Daylight" just because we could also understand them as something that augments the basic daylight analysis.

I might have suggested that they be in their own tab but 4 components seems a bit small for a tab and it would take a LONG time to move all of the other tabs around.

I'm leaning towards the first suggestion of renaming the Sky tab. Let me know your thoughts.

If anyone else has an opinion, please express it! -Chris

sariths commented 8 years ago

Hi @chriswmackey, I like the "02|Daylight| Light Sources" option too.

Anyways, I think people who want to find them will locate them regardless of wherever they are. I have intentionally started the name of every component with "IES.." so that it is easy to locate in the type-search component thingy on the Grasshopper canvas. So I will leave it to you and the BDFL (@mostaphaRoudsari ) to take a call on this. I totally agree that the number of components are too less for them to deserve a tab of their own. I want to add some dimming options moving forward but I am not sure when I will get to them. I should have more time once I am done with my stint at LBNL next month.

Actually with a few more additions to the code, we could be pretty close in functionality to both SPOT as well as DaysimPS. Porting it to HoneybeeX should be fairly easy to as I don't have too many dependencies from the Honeybee_Honeybee script except for the sticky dictionary.

mostaphaRoudsari commented 8 years ago

@chriswmackey @sariths though it's irony to have electrical lighting components under Daylight let's give them a separate section under 02|Daylight|Light Sources. I think we will give them their own tab in honeybeeX.

chriswmackey commented 8 years ago

@sariths and @mostaphaRoudsari ,

I like all aspects of the plan. I'll move them to "02|Daylight| Light Sources" once I get the chance and then close out the issue.

Before doing this, I have one feature request, though. I have been using the components to evaluate whether a project at my office passes the LEED Light Pollution credit and I have a list of ~20 exterior fixtures each with a slightly different orientation. I see that the 'IES Luminare Zone' component only allows you to assign a single orientation (or tilt, spin, aiming point) for all of the points in the zone. Since I have no interest in copying the zone component 20 times, I would very much like to be able to assign a list of orientations that correspond to the point list. I'm imagining that this could be implemented in a way that, if you have one value for orientation, it applies it to all points but, if you have a list that matches the length of points, it assigns each point a different orientation.

Let me know if you would be able to send a pull request for this in the next couple of days, If not, I can implement this as I need it for my work.

Let me know what you prefer, -Chris

sariths commented 8 years ago

@chriswmackey this...

Since I have no interest in copying the zone component 20 times, I would very much like to be able to assign a list of orientations that correspond to the point list.

...makes sense to me and your approach for implementing it also appears to be correct. I didn't really think of perimeter lighting or pathway lighting when writing the code.

I would suggest that you ahead and implement the change as it will be a while before I get access to a Rhino/Gh on a desktop. I have a trial version on my laptop that I had installed when coming to California but it eats up all the other resources on my system if I run it.

chriswmackey commented 8 years ago

@sariths ,

Ok. I will implement this capability tonight and report back here once done. I have one (hopefully final) question:

A number of the .ies files that I am trying to use give me the error: 'The luminous dimensions are neither rectangular nor circular. This is not supported currently.' However, if I comment out the part of the code that is giving me the warning, I get illuminance results from the simulation that seem to make sense. I just want to understand why there are a number of luminous dimensions that are not currently supported or if I could add in the necessary checks for the file type that I am using (for an exterior circular fixture with a cutoff). Here's the .ies file that I am trying to run in case this helps you answer the question better: https://www.dropbox.com/s/yemfqzmdkz9e7c8/SP1-IND--FCO-100HPS.IES?dl=0

These are all amazingly wonderful components, by the way. Awesome job!! -Chris

sariths commented 8 years ago

Hi @chriswmackey ,

In line 14 of the ies file, -1.83,-1.83,2.43 correspond to the luminous dimensions. The conventions followed for these numbers are given in the table (from LM-63) below: 7-27-2016 1-49-20 pm The shape in your IES file corresponds to a vertical cylinder. Mostly due to my limited skills with Rhino, I have only catered for shapes that are rectangular or circular. I think ies2rad, the main Radiance workhorse of my components, should be able to handle all kinds of shapes. So, to answer your question, the simulation should work but the display on the view port will not represent a cylinder as I haven't coded it yet. But you are most welcome to add that part..(and let me learn some more Rhino in the process ! :) )

chriswmackey commented 8 years ago

@sariths ,

Thanks for the explanation. I understand exactly what your thought process was with restricting the types of luminares that can be run because I implemented a lot of similar restrictions when I first built out the THERM workflow. However, I soon realized that many of the users are smart and can fill in the gaps where our code is not yet complete. As a result, I found that it is better practice in these cases to just give a warning to the user rather than stopping the component from running because the latter kills the whole funcationality of the components while the former only slows the workflow (and maybe even educates the users about what is going on). So, in this case, I would recommend that we just give a warning that we can't generate the light geometry but we still let the component run and generate the lunimare object for the simulation.

Let me know if you agree and, if so, I can implement this along with the proposed changes. Also, I have grown to know rhinocommon pretty well such that I can create many of the light geometries in your table. Let me know if you want me to give this a shot.

Finally, I just wanted to understand one final thing (I promise this is final now): Why is the luminareID a required input when you say on the description that "The default name is the same as the name of the IES file." If there is a default, then this should not be a required input. Was it that this turned out to be a bad default after you implemented it?

-Chris

sariths commented 8 years ago

@chriswmackey

Let me know if you agree and, if so, I can implement this along with the proposed changes. Also, I have grown to know rhinocommon pretty well such that I can create many of the light geometries in your table. Let me know if you want me to give this a shot.

Yes please! (to the warning message and light geometries).

luminaireID was a case of bad default. The luminaireID is required because we would like to prevent the user from overwriting the same file in case the same ies file is used in two different ways in the same simulation.

For example, lets say that someone is simulating a colorfully lit facade where the same flood-light is used in multiple locations with different color filters. So, in this case the multiplying factor, color values etc for each luminaire rad file will be different. Therefore we need the names to be unique. Secondly, if my memory serves me right, the luminaireID is also used in the BOQ component. We could do some autonumbering to avoid this but then it might get confusing if there are too many luminaires. Asking the user to assign a string as luminaireID seems to be a small price to pay..

chriswmackey commented 8 years ago

I am just posting this as a good reference on all of the luminous geomtry types: http://docs.agi32.com/PhotometricToolbox/Content/Indoor_Report_Tool/IES_Indoor_Report_Summary.htm

chriswmackey commented 8 years ago

@sariths ,

I have implemented all 14 of the light geometries on the Luminaire component: https://github.com/mostaphaRoudsari/Honeybee/commit/f5c48fe027314530956aaa97e06977c1e23817d7

You can test out all of them with these mock ies files that I generated here: https://www.dropbox.com/s/l24zfqo5zqdvk6l/DifferentDim.zip?dl=0

I can see how it was largely an exercise in Rhinocommon, which can take some time to master. Hopefully, we should now be able to run nearly all of the ies files that are out there in the industry with the exception of Type B photometry ones.

Also, I hope that you don't mind that I took out the luminaireID input and replaced it with a unique one that is generated each time the component is run. I am using the uuid module for this as you see in the commit.

I will move the components out of WIP once I get home in an hour unless you have any feedback on the changes. We are coming down to the final stretch. Looking forward to it!

-Chris

chriswmackey commented 8 years ago

@sariths , I moved the components out of WIP. Congratulations on a wonderfully useful contribution!!!

sariths commented 8 years ago

@chriswmackey All 14 of them?! Well.....Bravo!! I would have never gotten around to doing them, not in one sitting any way.

I don't have proper access to Rhino/Gh right now so I don't remember the exact name, but are you using the IES BOQ component? I think the reason I required the the luminaireID thing to be input was because I was using that to generate BOQs as well. I totally see your point of having the luminaireID being generated through uuid, but does that imply that people cannot name them if they want to?

By the way, we can do Type B photometry too. The issue there, again is with the Rhino viewport and not so much with ies2rad. For the code to be cleaner Type B photometry needs to be converted into Type C (which is doable and totally 'legal'). I didn't attempt it back in January as Type B photometry isn't all that common and most lighting engineers are aware of software which can do this conversion..

chriswmackey commented 8 years ago

@sariths ,

You are correct that the edit that I made currently does not allow you to assign a custom ID to the luminaire and I agree that we should leave the option to the user to set this name if they want to make the reading of the BOQ easier (much in the same way that the names of HBSrfs or HBZones is optional but setting them can make the interpretation of results easier). However, the component already has an input for customLumName and I am wanted to ask you why this input is not used as the optional name. Is there any reason why there need to be separate customLumName and customID inputs? If not, would you be in favor of combining these into one input?

Good to know about Type B photometry. I am a bit spent after inputting the 14 geometries yesterday but if you ever feel the urge to implement it or find someone who needs it, let me know and I can help with some of the Rhino part.

-Chris

sariths commented 8 years ago

@chriswmackey I remember that I had added the customLumName in response to certain IES files not having any name tags at all. But those names aren't the same as assigning two different names to the same luminaire. For example, a luminaire X can be used for illuminating a pathway as well as a boundary wall with lamps a and b respectively. My idea was that the user should be able to name the two luminaires as Xa and Xb (and so on) and have them reflect separately in the bill of quantity even though they are basically the same IES defintion.

I think your suggestion on combining the customLumName and customID is great, provided we can make it work. Since UUID already takes care of any potential overwrites and also assigns names automatically, having customLumName as an optional input should take care of all cases.

I can't run any simulations right now, however, I will test this out after I get back to Pennsylvania next week.

chriswmackey commented 8 years ago

@sariths ,

Sounds good. I will try to implement this tonight and there is no pressure with running simulations. Thank you for being so responsive during your travels.

-Chris

chriswmackey commented 8 years ago

@sariths ,

I edited the component such that it will now use the customLumName as the LuniareID. I also checked to be sure the the customLumName is inthe BOQ: image

https://github.com/mostaphaRoudsari/Honeybee/commit/ed215321c48199f0901140c02f73e2ab79656aef I am closing out the issue now but feel free to reopen if this is not as you intended.

-Chris

sariths commented 8 years ago

Hi @chriswmackey , there is a one line bug in the Honeybee_IES Luminaire component. In line 819 can you please change _iesFilePath = filePath to _iesFilePath = _iesFilePath[0].

On a related note, there is a "hidden" feature that you might not be aware of. Users need not link IES files via path. They can also embed them directly via text boxes as shown below: 8-30-2016 5-59-18 pm Its extremely rare for an IES file to be anything more than a couple of KB in size. So, it made sense to me at the time to enable a functionality where a IES definition can be directly embeded into the gh file.

mostaphaRoudsari commented 8 years ago

@sariths is the bug still there? should we re/open an issue for that and get it fixed?

sariths commented 7 years ago

@mostaphaRoudsari Yup. I actually forgot all about it since put this note here. I will open an issue for this. While on it, let me see if I can implement Type B photometry as well (we don't support that at present).