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

Added Electrolyzer Physics Tests #275

Merged

Conversation

elenya-grant
Copy link
Collaborator

Added GreenHEART Electrolyzer Physics Tests

Added on and off-grid electrolyzer physics tests

Related issue

Impacted areas of the software

Files Updated:

  • greenheart/tools/eco/electrolysis.py
  • tests/greenheart/test_hydrogen/input_files/plant/greenheart_config.yaml

Files Added:

  • tests/greenheart/test_hydrogen/test_electrolyzer_physics.py
  • tests/greenheart/test_hydrogen/input_files/plant/GS_greenheart_config.yaml
  • tests/greenheart/test_hydrogen/input_files/plant/GS_gridonly_power_signal.csv
  • tests/greenheart/test_hydrogen/input_files/plant/GS_offgrid_power_signal.csv

Additional supporting information

Part of land-based and offshore green steel code coordination and sync-up tasks. Test values are based on grid-only and off-grid land-based electrolyzer results.

Test results, if applicable

Copy link
Collaborator

@jaredthomas68 jaredthomas68 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good. Thank you Elenya! I left a few comments, but they should all be pretty easy to address.

greenheart/tools/eco/electrolysis.py Outdated Show resolved Hide resolved
greenheart/tools/eco/electrolysis.py Outdated Show resolved Hide resolved
greenheart/tools/eco/electrolysis.py Outdated Show resolved Hide resolved
greenheart/tools/eco/electrolysis.py Outdated Show resolved Hide resolved
@jaredthomas68
Copy link
Collaborator

Also, please make sure all checks are passing before we merge this.

Copy link
Collaborator

@jaredthomas68 jaredthomas68 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will do.

greenheart/tools/plant_sizing_estimation.py Outdated Show resolved Hide resolved
greenheart/tools/eco/electrolysis.py Outdated Show resolved Hide resolved
greenheart/tools/eco/electrolysis.py Outdated Show resolved Hide resolved
greenheart/tools/eco/electrolysis.py Outdated Show resolved Hide resolved
greenheart/tools/eco/electrolysis.py Outdated Show resolved Hide resolved
greenheart/tools/eco/electrolysis.py Outdated Show resolved Hide resolved
@jaredthomas68 jaredthomas68 self-requested a review March 12, 2024 19:45
Copy link
Collaborator

@jaredthomas68 jaredthomas68 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks close. I left a few comments, mostly implementation/syntax/comment related.

greenheart/tools/eco/electrolysis.py Outdated Show resolved Hide resolved
greenheart/tools/eco/electrolysis.py Outdated Show resolved Hide resolved
greenheart/tools/eco/electrolysis.py Outdated Show resolved Hide resolved
greenheart/tools/eco/electrolysis.py Outdated Show resolved Hide resolved
greenheart/tools/plant_sizing_estimation.py Outdated Show resolved Hide resolved
greenheart/tools/plant_sizing_estimation.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@jaredthomas68 jaredthomas68 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your good work on this Elenya, just a few comments.

@jaredthomas68 jaredthomas68 self-requested a review March 28, 2024 18:04
Copy link
Collaborator

@jaredthomas68 jaredthomas68 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will do. Thank you @elenya-grant!

@jaredthomas68 jaredthomas68 merged commit 6d93aea into NREL:greensteel-eco-sync Mar 28, 2024
4 checks passed
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

Successfully merging this pull request may close these issues.

None yet

3 participants