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

dev to main for 2024 release #19

Merged
merged 16 commits into from
Jun 25, 2024
Merged

dev to main for 2024 release #19

merged 16 commits into from
Jun 25, 2024

Conversation

brtietz
Copy link
Collaborator

@brtietz brtietz commented Jun 25, 2024

Major changes include:

  • Split offshore wind into fixed and floating
  • Add optional variables for which tax credit a technology is taking
  • Use these variables to allow PV+Battery to take different credits for the PV and battery components
  • Additional tech detail changes for Nuclear and Natural Gas to match the new ATB
  • Updated financial assumptions

brtietz and others added 14 commits January 13, 2024 10:41
* Add pumped storage hydropower one reservoir processor

* Update Nuclear to 2024 CFF calcs, one per tech detail

* Update new PTC equations for PV+Battery

* Proof of concept debt fraction calcs for PV+Battery

* Updates for TaxCreditCase field

* correct coal num_tds to account for igcc w/ ccs (#4)

* correct coal num_tds to account for igcc w/ ccs

* Add coal cases, commit errata data

* Automated scraping of different tax credit cases for pv+battery

* Add new pumped storage tech processor to example workbooks

* Updates for nuclear to start in 2030

* fix classmethod designation for nuclear cff, update test data for mid-flight 2024 data

* propigate tax credit case to abstract extractor definition for tests

* add additional argument to the mock class to fix tests

* clean up class definitions, only mock needs extra argument

* Fix minor type hint bugs (#7)

* Fix debt fraction calculator to read in tax credit case from file, also handle subset of dates for Nuclear. Some PR comments addressed

* docstring to address pr comments

* Get tax credit case working in the final flattening steps

---------

Co-authored-by: Mike Bannister <mike.bannister@nrel.gov>
* GCC should only be counted once in pv plus battery lcoe, convention is to count it towards PV

* update tests given new equation

* Minor style and typing changes

* Update psh one new res name for mac compatability

* minor typing updates

---------

Co-authored-by: mbannist <mike.bannister@nrel.gov>
* Update metrics and boolean flags to scrape new capex fields for all battery scales

* Update minor blank lines style issue
* Correct default for pv plus battery in debt fraction calculator, update depreciation assumptions for 2024 atb

* Update test data for latest tax credit assumptions

* Fix CRP as string bug (#15)

* Fix CRP as string bug

* rename self._crp to remove ambiguity

* add missing docstring

---------

Co-authored-by: Mike Bannister <mike.bannister@nrel.gov>
* Split offshore wind into fixed and floating

* Test and notebook changes for fixed vs floating osw

* Correct docstric for better intellisense support
@brtietz brtietz requested a review from mikebannis June 25, 2024 14:29
@mikebannis
Copy link
Collaborator

A couple questions/comments about UtilityPVPlusBatteryProc.

  1. It uses a custom extractor, but that extractor is not being used in extract_test_data.py. Is UtilityPVPlusBattery being adequately tested?
  2. Since it uses a custom extractor, I'm tempted to say the extractor argument should be removed from the __init__() definition on line 115 of tech_processors.py, and just have it be hardwired as a class attribute. I don't think that will break anything in the repo. Alternatively, allow the extractor argument if necessary, and ignore it. That feels dirty and should be documented.

@mikebannis
Copy link
Collaborator

Looks good to me other than comments above

@brtietz
Copy link
Collaborator Author

brtietz commented Jun 25, 2024

A couple questions/comments about UtilityPVPlusBatteryProc.

1. It uses a custom extractor, but that extractor is not being used in `extract_test_data.py`. Is UtilityPVPlusBattery being adequately tested?

Good question. The reason for the custom extractor is to allow for specifying the tax credit case during processing. So we're adequately testing the PTC+ITC case, but not the "ITC only" case. We'd have to do some upgrades to extract_test_data.py and test_lcoe_calculator.py to cover both cases.

2. Since it uses a custom extractor, I'm tempted to say the extractor argument should be removed from the `__init__()` definition on line 115 of `tech_processors.py`, and just have it be hardwired as a class attribute. I don't think that will break anything in the repo. Alternatively, allow the `extractor` argument if necessary, and ignore it. That feels dirty and should be documented.

Would this also require a change to base_processor.py line 107?

Thoughts on handling these in this PR vs making a separate issue? I can see some advantages to merging a little sooner in case users try to use main with the new spreadsheet, but happy to wait to tag the release until this is resolved.

@mikebannis
Copy link
Collaborator

Your call on the necessity and timing for updating tests. Regarding changes for the custom extractor, see my response comment above on the tech_processors.py file.

@brtietz brtietz merged commit aaa6193 into main Jun 25, 2024
12 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

2 participants