Conversation
31649bb to
65a06ad
Compare
06eecb2 to
adbdf78
Compare
|
Hi all! Daniel asked me to add some comments here as I’ve been using Ben’s PostREISE branch while creating our Training Session 3 slides. First and foremost, Ben, Thank You! I have found this tutorial quite helpful and the plotting functions easy-to-use. Along the way, I’ve found at least one code block that didn’t work for me. If this (or any other) is operator error, my apologies.
|
|
We also discussed that it might be smart to not use the Python keyword 'map' as the function return names in the Plotting Scenario Data blocks
|
|
In the figure code block |
This has been fixed. Unfortunately, I forgot to update the docs in Dropbox @danlivengood before taking off |
This has been fixed too. |
|
In the block "calculate total emissions by generator type and bus," I believe emission_by_resources_and_bus = summarize_emissions_by_bus(emission) should be emission_by_resources_and_bus = summarize_emissions_by_bus(emission, grid) |
|
In the blocks I'm getting a "ValueError: invalid renewable resource(s): ", which I believe is coming from the inclusion of hydro. It runs properly when I change it to solar and wind. |
|
In the heatmap figure 'plot any time-series values using a heatmap where each column is one color-coded day', there is a mismatch between the output of the I'm assuming those should be the same (it runs correctly when they align) NameError Traceback (most recent call last) NameError: name 'summed_curtailment' is not defined |
|
In the emissions chart 'compare total carbon emissions by generator type for 1-to-n scenarios through bar charts' |
|
In 2 code blocks, the output is either empty or 0. They are correct, but for a 'new' user, it might cause a question of wondering whether the block ran correctly.
|
|
In Recommend changing the block to be |
danielolsen
left a comment
There was a problem hiding this comment.
I've gone through all the code, and left a couple small comments where things could be shined up a bit more, but I think this is a big improvement and ready to go in whenever you think it is.
| if ``labels`` has a different length than the number of passed scenarios. | ||
| :raises TypeError: | ||
| if ``labels`` is not an iterable. | ||
| if ``labels_size`` is not an int. |
There was a problem hiding this comment.
Do we want to follow the convention: always return the axes of the plot and provide option to not show the plot immediately after the function call? Same comment for all the plot functions being refactored here.
There was a problem hiding this comment.
The plotting functions that are in the tutorial and that don't have the axes returned and the show_plot parameters are:
plot_bar_generation_vs_capacityplot_bar_shortfallmap_carbon_emission_generatorandmap_carbon_emission_differenceplot_curtailment_time_seriesplot_n_scenariosplot_generation_time_series_stackplot_heatmapmap_interconnectionsmap_lmpplot_pie_generation_vs_capacityplot_powerflow_snapshot
Should I go ahead and refactor them?
894fa90 to
c3ad1eb
Compare
BainanXia
left a comment
There was a problem hiding this comment.
Great work! I think this is good to go at this point with two open problems to solve in future PRs
a) refactor plots following the convention: returning an axes object and giving the option for now showing it
b) deal with notebooks
based on our offline discussions with @rouille and @danielolsen
985106b to
ed58070
Compare
Pull Request doc
Purpose
What the code is doing
Refactor 3 map related modules -
postreise.plot.plot_carbon_map,postreise.plot.plot_interconnectionandpostreise.plot.plot_lmp_map- to:bokehandmatplotlibbuiltin functionalities (color bar, data normalization, etc.)postreise.plot.plot_statesmoduleIn other modules:
postreise.plot.plot_powerflow_snapshotmodulepostreise.plot.plot_carbon_barmore flexible, it can now takes n scenariosplot_statesfunction located in thepostreise.plot.plot_statesmoduleTesting
Copy/Paste the code snippets in the docs.
Where to look
The
postreise.plot.plot_carbon_map,postreise.plot.plot_interconnection,postreise.plot.plot_lmp_mapandpostreise.plot.plot_carbon_barmodules. For the tutorial I would recommend to generate the docs (see below)Usage Example/Visuals
Two options:
ben/postreisebranch in the docs repository and runtoxthere. Click on PostREISE in the side bar and navigate the tutorial. The code snippets can be used to generate the plots that are rendered on this pagepostreise/plot/demo/plot_tutorialnotebook that has been used to generate the plots in the tutorial (I will delete the notebook before merging)Time estimate
1h
Note
I did not update the notebooks. Do we want to keep the notebooks once all the plots are in the docs? In fact, someone looking at the notebooks on GitHub won't be able to visualize the maps.