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

Addresses overly optimistic wind capacity factor issue #36

Merged
merged 14 commits into from
Sep 4, 2024
Merged

Conversation

samgdotson
Copy link
Member

Important

This PR should be reviewed after #34 has been merged.

This PR closes #35 by adding a parameter to the config.yml file called capacity_factor. The add_electricity rule modifies the wind availability time series by renormalizing to the capacity_factor value.

@samgdotson samgdotson added Type:Bug Something is wrong or broken. This issue or PR is related to a bug in code. Comp:Input This issue has to do with the input component of the code or document. (input parameters, prep) Difficulty:1-Beginner This issue does not require expert knowledge and may be a good issue for new contributors. Priority:2-Normal This work is important and should be completed ASAP. Status:5-In Review This issue has been handled, and the solution is being reviewed. (probably as a PR) labels Sep 3, 2024
@samgdotson samgdotson added this to the 2nd Set of Model Runs milestone Sep 3, 2024
@samgdotson samgdotson self-assigned this Sep 3, 2024
@pep8speaks
Copy link

pep8speaks commented Sep 3, 2024

Hello @samgdotson! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 463:80: E501 line too long (86 > 79 characters)

Line 7:1: E402 module level import not at top of file

Comment last updated at 2024-09-03 14:34:03 UTC

Copy link
Contributor

@ssattler ssattler left a comment

Choose a reason for hiding this comment

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

I left 3 comments, let me know if you can't see them,
-Sandra

@@ -1,13 +1,13 @@
state_abbr: 'IL'
version: "5.0-test"
scenario: "no_growth_no_export"
Copy link
Contributor

Choose a reason for hiding this comment

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

Will you be doing a growth scenario in this analysis?

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, as discussed. The config file represents the easily tunable parameters. In the technical appendix we can have a table describing the value of each (or a select set) parameter for a particular scenario.

n.generators.carrier).sum().T

return p_by_carrier


def plot_dispatch(n, year=2025, month=7):
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll talk about this on Thursday I'm sure, but as we are producing products for this analysis we may not want to show area plots - we might want to show bar charts or line charts instead (depending on the output). But we can go into more detail about that later.


return fuel_costs


def load_existing_generators():
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still a little confused on how you're dealing with "no exports" since the data that you have for existing generators included exports in that past year. Is this an issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

"Exports," here, just means that the total "demand" is equal to the total historical generation (since IL is an energy exporter). Removing "exports" means the total demand is equal to the historical retail sales within IL (about 2/3 of total generation).

@samgdotson
Copy link
Member Author

samgdotson commented Sep 4, 2024

@ssattler if you're satisfied with the changes in this pull request, would you mind hitting the big green "merge pull request" button? In general, it's best practice for somebody else (besides the person making the PR) to merge a pull request.

For chronological reasons, this PR needs to be merged after #35. PRs are listed newest to oldest by default, so this one appears to be "first" on the list. Thanks!

@samgdotson samgdotson mentioned this pull request Sep 4, 2024
@ssattler ssattler merged commit c3b2424 into main Sep 4, 2024
@samgdotson samgdotson deleted the wind_cf branch September 4, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Comp:Input This issue has to do with the input component of the code or document. (input parameters, prep) Difficulty:1-Beginner This issue does not require expert knowledge and may be a good issue for new contributors. Priority:2-Normal This work is important and should be completed ASAP. Status:5-In Review This issue has been handled, and the solution is being reviewed. (probably as a PR) Type:Bug Something is wrong or broken. This issue or PR is related to a bug in code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Wind availability is far too high to represent the average IL wind farm.
3 participants