-
Notifications
You must be signed in to change notification settings - Fork 17
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
Plots #84
base: develop
Are you sure you want to change the base?
Plots #84
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
examples/01-green-hydrogen.ipynb
Outdated
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.
@johnjasa can you remind me, did you say something about not pushing results in Jupyters?
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 you are right @kbrunik. @RHammond2 I pushed these results since there were new figures in the example. What is the correct/accepted approach on this?
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.
@@ -419,7 +419,7 @@ def visualize_plant( | |||
onshorex = 50 | |||
onshorey = 50 | |||
|
|||
wind_buffer = np.min(turbine_x) - (onshorey + 2 * rotor_diameter + electrolyzer_side) | |||
wind_buffer = np.min(turbine_x) - (onshorey + 3 * rotor_diameter + electrolyzer_side) |
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.
Why did this change?
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 needed a larger buffer so I called the buffer by an extra rotor diameter to avoid overlap in the wind/solar farms
greenheart/tools/eco/utilities.py
Outdated
@@ -889,10 +892,10 @@ def add_turbines(ax, turbine_x, turbine_y, radius, color): | |||
i = 0 | |||
for x, y in zip(turbine_x, turbine_y): | |||
if i == 0: | |||
elable = "Electrolyzer" | |||
elabel = "Electrolyzer" |
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'm wondering if we want to change this to "H$_2$ Electrolyzer" or "PEM Electrolyzer" since we are working on additional types of electrolysis (iron, alkaline, etc)
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.
Good point. I've changed it to "H$_2$ Electrolyzer" in all instances for now but am open to further changes.
label="Solar Array", | ||
hatch=solar_hatch, | ||
) | ||
if design_scenario["wind_location"] != "offshore": |
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.
Should this be an if statement for "pv_location"?
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.
No, there are different plots to add solar to depending on if wind is onshore or offshore. Convoluted and should be fixed, but I think that making this plot more clear/robust should wait for another PR.
# ax[ax_index_wind_plant].autoscale() | ||
ax[ax_index_wind_plant].set(aspect="equal") | ||
# ax[ax_index_wind_plant].xaxis.set_major_locator(ticker.\ | ||
# MultipleLocator(np.round(point_range_x*0.5, decimals=-3))) | ||
# ax[ax_index_wind_plant].yaxis.set_major_locator(ticker.\ | ||
# MultipleLocator(np.round(point_range_y*0.5, device_spacing=-3))) |
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.
Here and in a few spots below here, do you intend to keep these commented lines on purpose?
Include plotting functionality for greenheart output
This PR provides a few plotting options based on the
energy_flows.csv
data output from greenheart to visualize the electricity and hydrogen dispatch/flow/production independently and together. There are certainly better ways this could be done, but I'm not sure how far to go given the impending changes.PR Checklist
CHANGELOG.md
has been updated to describe the changes made in this PRdocs/
files are up-to-date, or added when necessaryRelated issues
Impacted areas of the software
examples/01-green-hydrogen.ipynb
: add example of using the new plotsgreenheart/tools/eco/utilities.py
visualize_plant
: adjustments for slightly more automatic scaling and better looking plots for onshore and offshoregreenheart/tools/plot.py
get_hour_from_datetime
plot_hydrogen_flows
: new functionplot_energy_flows
: new functionplot_energy
: new functionAdditional supporting information
Test results, if applicable