santoshphilip / eppy

scripting language for E+, Energyplus
MIT License
151 stars 67 forks source link

ex_loopdiagram fails on IDFs exported from DesignBuilder #31

Closed jamiebull1 closed 9 years ago

jamiebull1 commented 9 years ago

DesignBuilder generates zone names by joining the building name and zone name with a colon (e.g. MyHouse:SAPZone1). Colons break the digraph formatting which shows as an error when eppy calls pydot.create(). I'm not sure whether the solution is to replace colons with dashes after importing the IDF inside ex_loopdiagram.py or to advise the user to do so manually in the Eppy documentation (and potentially raise an error with this advice if the problem arises) but it's likely to be a common problem.

santoshphilip commented 9 years ago

I am of the opinion that it should replace the colons with dashes and then give a warning message. I think it should be straight forward to do that (just slightly tricky :-).

Jamie, would you be interested in making that update. I can help as needed.

jamiebull1 commented 9 years ago

Take a look at (and possibly reject) the pull request I made earlier. Is there a better way to do this? I had a problem when trying to do it by just changing the data dictionary.

santoshphilip commented 9 years ago

Jaime, Will take a look.

I have sense of how to do this. I'll write some quick pseudo code to see if my way is viable. I had put some thought into this many months back when I first ran into this problem.

One thing to take care of is that if you change the name of the object, one has to change it all the places it is referred to. (other objects may refer to that object).

santoshphilip commented 9 years ago

To be honest, the loop diagram maker is a bit of a hack. I wrote it when I desperately wanted to see what the loop looked like. At some point it would be worth testing on all the example files and see where it does not work.

In any case, that would be for the future opened as a different issue

jamiebull1 commented 9 years ago

I had done that but there were still references getting into the .dot file. That's why I ended up taking the decision to use a temporary IDF but I'm sure you can see why the previous way wasn't working. I'm pretty sure my recursive replacement approach would have picked up all the places it needed replacing.

Anyway, hack or not, its a really useful tool. It would be good to get it polished up. On Oct 19, 2014 8:23 PM, "santoshphilip" notifications@github.com wrote:

Jaime, Will take a look.

I have sense of how to do this. I'll write some quick pseudo code to see if my way is viable. I had put some thought into this many months back when I first ran into this problem.

One thing to take care of is that if you change the name of the object, one has to change it all the places it is referred to. (other objects may refer to that object).

— Reply to this email directly or view it on GitHub https://github.com/santoshphilip/eppy/issues/31#issuecomment-59661436.

santoshphilip commented 9 years ago

made a branch "i31_loopdiagram" to work on this. I'll work with this for some time and do a handoff to you if it makes sense at that point.

santoshphilip commented 9 years ago

Jamie, I have made you a collaborator. Can you do the following ?

jamiebull1 commented 9 years ago

Done. I've added Boiler.idf in "/eppy/eppy/resources/idffiles/V8_1_0" and also the required IDD in "/eppy/eppy/resources/iddfiles/V8_1_0".

On 19 October 2014 22:40, santoshphilip notifications@github.com wrote:

Jamie, I have made you a collaborator. Can you do the following ?

  • checkout branch "i31_loopdiagram"
  • get hold of a design builder file that has colons in the zone names
  • put this file into the folder "/eppy/eppy/resources/idffiles"
  • commit and push to github
  • I'll use that file to experiment

— Reply to this email directly or view it on GitHub https://github.com/santoshphilip/eppy/issues/31#issuecomment-59666882.

santoshphilip commented 9 years ago

Jamie, I may have a solution. It was trickier than I anticipated. In any case here is what I have:

This script reads the Boiler.idf and generates four files:

  1. bl_original.idf
  2. bl_renamed_names.idf
  3. bl_renamed_fields.idf
  4. bl_renamed_nodes.idf

The final file "bl_renamed_nodes.idf" will generate a clean loop diagram. Use a diff program and compare the above 4 files. You will see what renamecolon.py did.

santoshphilip commented 9 years ago

I think there is a cleaner way to do this. I am going to try do the rename within loopdiagram.py (should have thought of this first :-(

If that does not work, we can always fall back on solution embodied in renamecolon.py

santoshphilip commented 9 years ago

these changes are in the develop branch

OK … I got a much cleaner fix by making changes in loopdiagram.py It took about 20 lines to do this :-) I have marked it with a comment ----------changes to fix the Designbuildier file problem---------------- so you can find it

This should work for all Designbuilder files now. Jamie, Can you test it with a few other files ?

I would like to give some warnings when loop builder changes the file names

jamiebull1 commented 9 years ago

Looks a lot cleaner than my approach. I'm getting an error on line 199 in parse_idd when it hits the first line without data though.

, !- Dry-Bulb Temperature Range Modifier Type

if elm[-1]==';': raises an IndexError.

santoshphilip commented 9 years ago

That error is unexpected and rather troubling Can you upload the file that give the error (I assume it is not the Boiler.idf)

which version of eppy are you using (give me the commit number if you are pulling it off gihub - use "git log" to see the commit number)

jamiebull1 commented 9 years ago

Ah, I've spotted my mistake. I had the run configuration set up in Eclipse with the IDF as the first arg and the IDD as the second instead of the other way around. I've tried it out with a few IDFs from DesignBuilder now including one with a lot of different systems. No more problems so I'd say this can be closed, unless there's anything else you'd like to happen.

santoshphilip commented 9 years ago

Thanks Jamie, I'll go ahead and close this issue.

I noticed that I am using code written by you deep within eppy. See eppy/geometry/surface.py see the function "area".

I got that code from <http://code.activestate.com/recipes/578276/ > (and I have credited that link) I noticed the link http://oco-carbon.com/coding/python-and-energyplus-polygon-areas-in-3d-space/ and realized that you had written it.

Can you send me an email at santosh_philip@yahoo.com to continue this conversation ?

santoshphilip commented 9 years ago

loopdiagram should give a warning when it changes the object names. I'll open a new issue on this