-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Lcoe branch #1687
base: main
Are you sure you want to change the base?
Lcoe branch #1687
Conversation
Financial module - includes LCOE function and supporting financial functions - with unit tests and two examples. |
Great start @eccoope. Very happy to see your contribution. There are a number of stickler issues. Can you please resolve them so it will be easier to review? |
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.
You might consider converting to draft?
mikeofski recommended not rounding returns in the financial.py module, tests needed to be updated for expected to match out
A few more stickler errors. I know they're a drag, that's why it's called the "stickler" 😆 Also, tiny nit, not a huge blocker, but please try to use a message for every commit. Even better if it's unique and meaningful. Thanks! These messages can help in reviews. 😄 |
Debt tenor was changed to analysis period as recommended by @brtietz, discount rate was clarified to be real
in lcoe_sam_validation.py
financial.py
financial.py
per @cwhanse 's recommendation
@eccoope please add a line to https://github.com/pvlib/pvlib-python/blob/main/docs/sphinx/source/reference/index.rst so that the new page builds in the API reference. |
@eccoope you'll need to resolve the conflict in the "What's new" for v0.9.5. This happens often unfortunately because it's one of the few files that gets edited simultaneously by more than one PR. In this case I think it's easy to fix -- you just want your name to appear AND the other two contributors as well who are already in the main branch's version of It's entirely possible that just sync'ing your main branch, merging it into this |
Hi @eccoope, to have more time to build consensus on this PR, we're pushing it to the next version so that v0.9.5 can be released now. This will mean some editing your part to update the What's new entry for v0.10.0. Thanks! |
@eccoope when you get a chance, can you merge master into your branch, resolve the what's new conflicts (sorry this always happens) maybe move them to v0.9.6 or v0.10.0, and resync? thanks! I am hoping we are all on schedule to push this in the next major release? I could use it even sooner. |
pvlib/financial.py
Outdated
Real levelized cost of electricity (LCOE). | ||
|
||
Includes cost of capital and fixed operations and maintenance (O&M), but | ||
not variable O&M. Described in [1]_, pp. 43 and 47-48, and defined here as |
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.
something I learned recently is that the variable o&m cost is zero for solar and typ. refers to the cost of fuel for fossil fuel power
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.
also, if I understand correctly both the simple and fixed charge rate LCOE models assume that the degradation rate is the same as the discount rate or the cost of capital, which is probably conservative. Deg rates are typically less than 1% while the suggested discount rate is 3%. The FCR simplification is:
LCOE = (FCR*CAPEX + OPEX_FIX) / YIELD + OPEX_VAR # OPEX_VAR = zero for solar!
which is equivalent to:
LCOE = (CAPEX + OPEX/FCR) / (YIELD/FCR) # just use OPEX for fixed O&M
I believe we can do better than this by just using either the lifetime production in the denominator or using a different FCR for production:
LCOE = (CAPEX + OPEX/FCR) / (YIELD/FIXDEGRATE)
# where FIXDEGRATE = [1 / SUM(1 / (1+RD)**n) for n in range(N)]
# N is lifetime in years, same as used for fixed charge rate
# and RD is the degradation rate, usually 0.75%/year
I think this might return an LCOE with more realistic accounting for degradation?
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.
The LCOE models apply the financial discount rate to the future energy yield, see equation 4-6 of https://www.nrel.gov/docs/legosti/old/5173.pdf This allows future costs and future energy to be on the same basis.
PV degradation needs to be included in the yield number. For a discounted cash flow we use equation 4-6 (the sum with specific values for each year). For the simple LCOE formula the ATB uses an average capacity factor over the life of the system.
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 didn’t realize the FCR applies to the energy as well, I guess it makes sense that it’s value changes over time because of inflation.
okay so then the average yield should be the total production including degradation divided by the lifetime of the system? Thanks!
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.
okay so then the average yield should be the total production including degradation divided by the lifetime of the system? Thanks!
That's correct!
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.
Not to dampen enthusiasm, but does the Short et al. reference include degradation? I know that including degradation may give a more meaningful LCOE, but here, we're implementing what's been published. The function can be extended or modified to include degradation but I'd like to see a second reference describing how that's done.
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.
@eccoope I think there should be something in the docs or example that shows that the fuel cost is zero for solar. Perhaps other newbies to financing like me will also be confused
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.
Not to dampen enthusiasm, but does the Short et al. reference include degradation? I know that including degradation may give a more meaningful LCOE, but here, we're implementing what's been published. The function can be extended or modified to include degradation but I'd like to see a second reference describing how that's done.
Looking at the code again, given that production is an array, it might be better to have users include degradation in that array. In that case, equation 4-6 for Short et al. still works. If you prefer a year 1 energy yield and a degradation factor, equation 2 of https://pubs.rsc.org/en/content/articlehtml/2011/ee/c0ee00698j provides a really good example.
If there is demand for the average yield approach I can ping the PV team for the ATB.
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.
Hi @mikofski ! Degradation rates are implemented as an average capacity factor from the ATB as mentioned by @brtietz in simple_lcoe_calculator.py (but I could probably be more explicit about this in a comment), but there's also a discount rate of 3% in this example which is separate from degradation.
Degradation is not accounted for in docs/examples/financial/lcoe_sam_validation.py - constant AC power is explicitly assumed (see line 126) - but we could easily incorporate by multiplying the production array element-wise with a degradation array like the one you defined as FIXEDEGRATE
.
@mikofski and @brtietz - if either of you would be interested in meeting with me and possibly @cwhanse for 30 minutes to clarify any of these details, let me know
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.
@eccoope Sure - please email me at brian.mirletz (at) nrel (dot) gov to set up a time.
is there any news on merging this, any required change or should it be impelmented on another lib to ease maintainability ? |
The submitter appears to have moved on to other work. My read is that there is still work to be done here before we get agreement to merge, so looking for a volunteer to adopt this PR. |
@cwhanse what are you envisioning for additional work, besides resolving merge conflicts as they arise? |
docs/sphinx/source/reference
for API changes.docs/sphinx/source/whatsnew
for all changes. Includes link to the GitHub Issue with:issue:`num`
or this Pull Request with:pull:`num`
. Includes contributor name and/or GitHub username (link with:ghuser:`user`
).remote-data
) and Milestone are assigned to the Pull Request and linked Issue.