Skip to content
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

Move metadata generation (Alfalfa Ruby gem) into core OS #506

Closed
anyaelena opened this issue Jun 4, 2024 · 6 comments
Closed

Move metadata generation (Alfalfa Ruby gem) into core OS #506

anyaelena opened this issue Jun 4, 2024 · 6 comments
Assignees

Comments

@anyaelena
Copy link
Member

For discussion: target 9/24 release of OS. Staffing on OS side?

@anyaelena
Copy link
Member Author

@TShapinsky @kbenne please document approach and pros/cons here.

@TShapinsky
Copy link
Member

TShapinsky commented Jun 14, 2024

Moving Alfalfa metadata generation library into OS SDK

Pros:

  • Ease of use: Users don't need to leave their modeling tools or even their modeling documentation to use
  • Portability: Models with Alfalfa points are runnable in vanilla os workflows.
  • Forward Looking: If generation was added to the C++ OS ruby and python measures would both be executing the same underlying code
  • Automatic version management: If breaking changes are made to the Alfalfa metadata generation the underlying code is automatically updated, the user does not need to pull code and manually set up additional environment.

Cons:

  • Slower cycles: fixes need to wait for an OS release
  • Greater breadth of expertise needed for project: developers on Alfalfa will need to understand OS to have full coverage of code

Keeping Alfalfa ruby gem as separate code

Pros:

  • Already exists: Wouldn't require any changes
  • Fast Fixes: fixes can be rolled out without any third party

Cons:

  • Not Portable: OS Workflows can only be run with alfalfa gem
  • Not Python Ready: additional python code needed for capability with new python measures
  • High Friction Setup: Modelers hoping to test locally need to fetch the alfalfa gem folder and figure out how to include it in their openstudio run command

Other thoughts

Right now models processed by alfalfa are not portable because the external interface of E+ requires a socket connection to tell it when to advance. Additionally the functions that the alfalfa gem accomplishes are not trivial because of how we've had to work around the limitations of the external interface.

Transitioning to pyenergyplus from external interface makes the job of adding points to alfalfa much simpler. This is because creating inputs and outputs will no longer require modifications to the underlying model. This means that the only thing that model needs for alfalfa is metadata telling alfalfa which points to expose.

I don't think it makes sense to move to alfalfa metadata in the OS SDK until we are in the process of migrating to pyenergyplus. This is because the functions needed to implement metadata generation will become much simpler and are less likely to require changes in the future. Additionally, before we transition to pyenergyplus the models post alfalfa aren't portable anyways due to the inclusion of external interface.

@anyaelena
Copy link
Member Author

This is added to our roadmap with the following timing:

  1. Updates to OpenStudio - can start immediately. Must be complete prior to October OS release which will include E+ 24.2 that includes required updates to Python bindings. Specifically, includes a way to access EMS global variables from Python. Confirm E+ roadmap with @Myoldmopar
  2. Release of Alfalfa using this functionality targeting FY25Q2 to allow time for OS release and any bug fixes.

@TShapinsky
Copy link
Member

After talking with @kbenne the alflalfa point list will be stored in the OSRunner and expose an API similar to what currently exists in the migrate_pyenergyplus branch.

@anyaelena
Copy link
Member Author

update from standup - @TShapinsky's current OS code reviewed by @kbenne. next steps to get this merged into OS:

  1. small adjustments requested by kbenne
  2. review of sample code/naming by @tanushree04 and @bonnema to ensure naming is intuitive for modelers
  3. assign someone on OS to review initiate merge of PR

@kbenne kbenne self-assigned this Sep 10, 2024
@anyaelena
Copy link
Member Author

Nice work @TShapinsky !
NREL/OpenStudio#5236

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants