-
Notifications
You must be signed in to change notification settings - Fork 460
Python HVAC-Diagram #9217
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
base: develop
Are you sure you want to change the base?
Python HVAC-Diagram #9217
Conversation
91509b5 to
dd4b407
Compare
JasonGlazer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I like the approach.
| The HVAC-Diagram program is a Fortran utility that can be used to generate | ||
| an SVG representation of the HVAC system in a model based on the BND file | ||
| generated by EnergyPlus. The program has limited flexibility and the SVG | ||
| output is not correctly rendered by all SVG rendering libraries. The SVG format |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What SVG rendering libraries do not work with the current HVAC-Diagram program?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is #6384. Internet Explorer still works, but Chrome and Edge are putting the labels are in the upper left corner. Presumably that's because Edge now uses the same thing that Chrome does.
|
|
||
| * One or more DOT language [2] files. This format is used by the | ||
| open source Graphviz package to display and manipulate graphs. Other formats | ||
| will be considered depending on development team feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the DOT approach but I am concerned that the resulting diagram will be too jumbled. Is their some way with the DOT format to specify the structure of the diagrams such that airloops are shown in one area and water loops in another with links between them?
BTW, I just used a similar approach of a Python script creating a DOT file here:
https://github.com/JasonGlazer/jsonSchemaToDot
and you can see that the result (in the PDF) can be a little jumbled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There some ways to set attributes of the graph, but rendering is usually going to be up to the program that does the rendering and I would expect that most renderings would be jumbled. Before the log4j thing happened, I usually used the Gephi program to look at graphs, and depending on the layout algorithm the output can be pretty messy. In any case, the DOT output isn't going to be the default and will definitely be more of an "expert" thing. I did it originally to make sure I was getting the connections out of the BND correctly and decided to leave it in.
|
|
||
| The BND output from EnergyPlus will be used to generate several forms of output: | ||
|
|
||
| * One or more DOT language [2] files. This format is used by the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if the diagram appears in multiple output files that correspond to multiple DOT files that it will be as useful to the user. Users are looking for a single diagram that explains all the HVAC connections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the programs that I have seen that can read DOT don't do great with multiple graphs in a single file, so adding some ability to control how much is in one file could be helpful, but not necessarily required.
| capabilities can be leveraged to produce semi-interactive representation of the | ||
| HVAC loops (e.g. simple controls to turn on/off node names or other ancilliary | ||
| information). Numerous visualization libraries are available (e.g. D3, p5.js) as | ||
| well as more traditional techniques (e.g. canvas, imagemaps). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Letting the user be able to zoom into part of the diagram that is important to them is good and an advantage to SVG. Can this be done with canvas or imagemaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it depends on how complex an approach is taken. If the Javascript path is taken, there're a lot of possibilities.
| the output of the original utility. | ||
|
|
||
| The functionality will be implemented as a Python class that is initialized with | ||
| input data (e.g. the BND file) and has the capability to produce the output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe for some complicated configurations, the diagrams won't render using the existing HVAC-Diagram because not all the connection information is in the BND file. Are you planning on enhancing the BND file as part of this effort?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly. The first priority is to get as far as we can with the current BND and then look to either enhance the BND or perhaps pull information from epJSON.
| -o file, --output file | ||
| write output to named file | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please make the default operation without options consistent with the naming convention currently used with HVAC-Diagram.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll do our best.
| format, but will require additional user effort to understand. The HTML output is | ||
| intended to be a more full-featured representation of the HVAC with interactive | ||
| features (optional display of node names, etc.) with fewer possibilities of bad | ||
| rendering. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interactive features would be great. Could you expand on this a bit? It would be great if additional information about the various connections could be shown.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, we can expand this.
|
@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated. |
6 similar comments
|
@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated. |
|
@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated. |
|
@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated. |
|
@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated. |
|
@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated. |
|
@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated. |
|
@jasondegraw I've cleared the 22.2 milestone on this PR. if you plan to resume work soon, leave it open. If not, please close the PR and reopen when ready. |
|
@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated. |
4 similar comments
|
@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated. |
|
@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated. |
|
@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated. |
|
@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated. |
|
@jasondegraw do you think this will be good for 23.1? Doesn't matter either way, just setting up the review queue and moving things to the right milestone. |
|
@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated. |
|
Quite interested in this feature. I'll keep an eye out for future updates... 👍🏻 |
|
@jasondegraw it has been 10 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
5 similar comments
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
2 similar comments
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 14 days since this pull request was last updated. |
|
@jasondegraw it has been 9 days since this pull request was last updated. |
1 similar comment
|
@jasondegraw it has been 9 days since this pull request was last updated. |
|
@jasondegraw it has been 8 days since this pull request was last updated. |
|
@jasondegraw it has been 14 days since this pull request was last updated. |
|
@jasondegraw it has been 12 days since this pull request was last updated. |
|
@jasondegraw it has been 8 days since this pull request was last updated. |
1 similar comment
|
@jasondegraw it has been 8 days since this pull request was last updated. |
|
@jasondegraw it has been 10 days since this pull request was last updated. |
|
@jasondegraw it has been 12 days since this pull request was last updated. |
|
@jasondegraw it has been 59 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
2 similar comments
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
11 similar comments
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
|
@jasondegraw it has been 7 days since this pull request was last updated. |
Pull request overview
The HVAC-Diagram tool that is currently packaged with EnergyPlus is written in Fortran. A new Python-based tool for generating HVAC topology diagrams and graphs is desired that improves upon the capabilities of the current tool (e.g. inclusion of nodes and other details) and is more flexible.
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.