Skip to content

Conversation

@jasondegraw
Copy link
Member

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.

  • Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
  • Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
  • Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
  • If any diffs are expected, author must demonstrate they are justified using plots and descriptions
  • If changes fix a defect, the fix should be demonstrated in plots and descriptions
  • If any defect files are updated to a more recent version, upload new versions here or on DevSupport
  • If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
  • If structural output changes, add to output rules file and add OutputChange label
  • If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • If branch is behind develop, merge develop and build locally to check for side effects of the merge
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally
  • Check any new function arguments for performance impacts
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

@jasondegraw jasondegraw added the NewFeature Includes code to add a new feature to EnergyPlus label Dec 29, 2021
Copy link
Contributor

@JasonGlazer JasonGlazer left a 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
Copy link
Contributor

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?

Copy link
Member Author

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.
Copy link
Contributor

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.

Copy link
Member Author

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
Copy link
Contributor

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.

Copy link
Member Author

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).
Copy link
Contributor

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?

Copy link
Member Author

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
Copy link
Contributor

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?

Copy link
Member Author

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
```

Copy link
Contributor

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.

Copy link
Member Author

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.
Copy link
Contributor

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.

Copy link
Member Author

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.

@mjwitte mjwitte added this to the EnergyPlus 22.2 milestone Feb 10, 2022
@nrel-bot-2b
Copy link

@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated.

6 similar comments
@nrel-bot
Copy link

nrel-bot commented Apr 8, 2022

@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated.

@nrel-bot
Copy link

nrel-bot commented May 6, 2022

@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated.

@nrel-bot
Copy link

nrel-bot commented Jun 3, 2022

@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated.

@nrel-bot-2c
Copy link

@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated.

@nrel-bot
Copy link

@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated.

@nrel-bot-3
Copy link

@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated.

@mjwitte mjwitte removed this from the EnergyPlus 22.2 milestone Sep 13, 2022
@mjwitte
Copy link
Contributor

mjwitte commented Sep 13, 2022

@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.

@Myoldmopar Myoldmopar added this to the EnergyPlus 23.1 milestone Sep 22, 2022
@nrel-bot-2
Copy link

@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated.

4 similar comments
@nrel-bot-2c
Copy link

@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated.

@nrel-bot
Copy link

@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated.

@nrel-bot-2c
Copy link

@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated.

@nrel-bot
Copy link

nrel-bot commented Feb 9, 2023

@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated.

@Myoldmopar
Copy link
Member

@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.

@nrel-bot
Copy link

@jasondegraw @Myoldmopar it has been 28 days since this pull request was last updated.

@manjavacas
Copy link

Quite interested in this feature. I'll keep an eye out for future updates... 👍🏻

@nrel-bot-2c
Copy link

@jasondegraw it has been 10 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

5 similar comments
@nrel-bot-2b
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2c
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2b
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2c
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

2 similar comments
@nrel-bot-2c
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2c
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 14 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 9 days since this pull request was last updated.

1 similar comment
@nrel-bot-2
Copy link

@jasondegraw it has been 9 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 8 days since this pull request was last updated.

@nrel-bot-2c
Copy link

@jasondegraw it has been 14 days since this pull request was last updated.

@nrel-bot-2c
Copy link

@jasondegraw it has been 12 days since this pull request was last updated.

@nrel-bot-2c
Copy link

@jasondegraw it has been 8 days since this pull request was last updated.

1 similar comment
@nrel-bot-2
Copy link

@jasondegraw it has been 8 days since this pull request was last updated.

@nrel-bot-2c
Copy link

@jasondegraw it has been 10 days since this pull request was last updated.

@nrel-bot-2c
Copy link

@jasondegraw it has been 12 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 59 days since this pull request was last updated.

@nrel-bot-2c
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

2 similar comments
@nrel-bot-2c
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2c
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@mitchute mitchute removed this from the EnergyPlus 24.2 milestone Oct 22, 2025
@nrel-bot-2c
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

11 similar comments
@nrel-bot-2c
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2c
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

@nrel-bot-2
Copy link

@jasondegraw it has been 7 days since this pull request was last updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NewFeature Includes code to add a new feature to EnergyPlus

Projects

None yet

Development

Successfully merging this pull request may close these issues.