Skip to content

Implement initial storage support #1378

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Implement initial storage support #1378

wants to merge 4 commits into from

Conversation

Peque
Copy link
Contributor

@Peque Peque commented Jan 15, 2022

  • Closes Add initial storage support #1333
  • I am familiar with the contributing guidelines
  • Tests added
  • Updates entries to docs/sphinx/source/api.rst for API changes.
  • Adds description and name entries in the appropriate "what's new" file in 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`).
  • New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
  • Pull request is nearly complete and ready for detailed review.
  • Maintainer: Appropriate GitHub Labels and Milestone are assigned to the Pull Request and linked Issue.

@williamhobbs
Copy link
Contributor

In the documentation here, it notes that "Timestamps are associated to the beginning of the interval". My understanding is that the default convention for most of pvlib is that timestamps are instantaneous. If interval-averaged data with beginning of interval timestamps will be the convention for storage (which make sense to me), it might be helpful to clarify that this is different from other parts of pvlib.

@mikofski
Copy link
Member

@Peque would you consider showing your work on storage at the SciPy conference?
https://www.scipy2022.scipy.org/

@Peque
Copy link
Contributor Author

Peque commented Jan 19, 2022

@williamhobbs Thanks for the comment! I will make sure it is more clearly explained. 😊

@mikofski Do you know if traveling is required for that conference?

@mikofski
Copy link
Member

Do you know if traveling is required for that conference

moved this to discussions: #1383

@Peque
Copy link
Contributor Author

Peque commented Feb 5, 2022

@cwhanse @wholmgren If you could have a quick look at this initial proposal, I would love to know about your opinion! 😊

The idea is to implement other models/dispatching algorithms but, before moving forward to more complicated things, I would like to make sure this is aligned with pvlib. 🚀

@cwhanse
Copy link
Member

cwhanse commented Feb 9, 2022

@Peque this is a good start. What would you think about pvlib.storage.battery (battery efficiency/state-of-charge models), with pvlib.storage.dispatch (functions that take in a Series of PV power and return some kind of optimal dispatch)? Musing if it makes sense to group storage-related code, but I don't have much sense for the breadth of functions yet.

About power vs. energy, I think we can make a simple assumption that power is assumed to be constant in each time interval, which makes conversion to energy easy. I'm OK with a left-labeled time convention, and the positive means discharge convention.

I have two hesitations. Although the PR doesn't have financial calculations, I sense we may be headed that way if we start coding dispatch optimization. I'm pretty sure that we don't want to start building financial calculations into pvlib. That capability is quite mature in SAM, and I for one am not enthusiastic about redundant capability, nor to learn all that stuff.

I am not sure about pvlib.flow (pvlib.powerflow is better). I think we'll want to set some clear boundaries for this module to manage expectations for this module. For example, I don't think we want to include interfaces for power flow software such as Open DSS, that's a lot to maintain.

@Peque
Copy link
Contributor Author

Peque commented Feb 15, 2022

@cwhanse Thanks for your feedback! I'll update the PR with your suggestions before moving forward and adding more code.

One question: should the "flow" module assume power flow or energy flow? Right now it is implemented as power flow (and SAM seems to be using that convention). That feels a bit weird to me (as opposed to energy flow), but if it is the most common convention, I'll leave it as-is.

I agree that financial calculations should not be the scope of pvlib. These would include things like payback, discount rate, IRR, replacement costs... However, I think price series could be supported as input of dispatch strategies. That means pvlib could support some kind of prize-based strategies. It could be as simple as "dispatch only when price is above X" or it could even be "dispatch only when price is above an optimal and automatically converged value". If the latter is something out-of-scope for pvlib (considering the optimization could be based on already-existing dependencies like SciPy), then I could maybe think about the interface (and add an example) on how to implement these types of dispatch strategies on your own. Any thoughts on this?

PS: not sure if the discussion should happen here or in the related issue (#1333). If the latter, then maybe we can copy these last 2 comments there, so that other people that have subscribed can participate. 😊

@janinefreeman
Copy link

We've gone with power flow rather than energy flow in SAM primarily because it is independent of time, which significantly simplifies calculations if you want to be able to model different timestep lengths.

@Peque
Copy link
Contributor Author

Peque commented Apr 25, 2022

I just pushed some changes and added a comment in #1333 to keep conversations not directly related to code implementation in the issue instead of the PR.

Sorry for the unplanned delay but we found ourselves under an unprecedented work load. Thankfully, that resulted in something positive: we have grown and are planning to further grow our team. That means, hopefully, we will be able to contribute more back to pvlib and SAM in the mid term. 🚀 😊

@Peque
Copy link
Contributor Author

Peque commented Nov 2, 2022

@cwhanse Where you able to take a look at the latest changes in this MR? (see #1333 (comment)) 😊

@mikofski
Copy link
Member

mikofski commented Nov 2, 2022

Hi @Peque sorry for the delay - I have been meaning to go through your PR and respond to your comments. Without having reviewed it yet, I can't say much. Although I was a little concerned that you said you rewrote some of the inverter modeling. I was understand the need to reverse some of the models, but I was hoping that we could reuse code in pvlib if possible. I was also very much in favor of an agnostic rules or dispatch based model, rather than a self optimizing one that integrates an objective function as part of the package. My reasoning is that

  1. the rules or dispatch based storage models have been documented in the literature, which is important in pvlib
  2. optimization can be a secondary function that wraps the rules based approach, IMO this is another important philosophy of pvlib, is that it's a toolbox not a polished solution. So we provide an implementation of the documented function, and then others, like your company, can take that function and wrap it in their own optimization routines.
  3. Wrapping a rules or dispatch based storage method in optimization routine (like those from scipy.optimize) allows more flexibility in the criteria - for example some may wish to optimize the charging rate for full on shifting, vs. others may want to optimize the dispatch schedule to leverage TOD pricing.

Given a rules or dispatch based storage function such as the one in SAM, then using scipy.optimize should be straight forward by defining an objective function, tolerances, boundaries, and starting guesses. EG:

from pvlib.storage import pvbess
from scipy.optimize import minimize

DISPATCH_SCHEDULE = pd.DataFrame(index=pd.daterange(start, end), [True, True, True, ..., False, False, False, ...])
PVSYSTEM = pvlib.pvsystem(location=pvlib.location(lat, lon, elev, tz), inverter, module, ...)
TOD = pd.DataFrame(index=pd.daterange(start, end), [0.14, 0.14, 0.14, ..., 0.07, 0.07, 0.07, ...])

def lcoe(dispatch, tod, pvsystem, max_cap, charge_rate, discharge_rate, aux_loss, ...):
    energy = pvbess(pvsystem, dispatch, max_cap, charge_rate, discharge_rate, axu_loss, ...)
    return energy * tod  # revenue


def objective(dispatch, tod, pvsystem, max_cap, charge_rate, discharge_rate, aux_loss, ...):
    return -lcoe*lcoe

best_dispatch = minimize(objective, DISPATCH_SCHEDULE, args...)

@Peque
Copy link
Contributor Author

Peque commented Nov 3, 2022

@mikofski Thanks for sharing your thoughts Mark! 😊

The current implementation always leaves the dispatch optimization to the user (i.e.: a dispatch series needs to be provided to run the simulation). Only, just like with AC-connected batteries, some constraints are enforced to make sure the energy flow is physically possible. These constraints may alter the original dispatch series.

Note that the new inverter model for DC-connected batteries is not defined in inverter.py, since it is meant to be an example for people wanting to try this non-official model. I reused as much code as possible, but it was the only way I found without having to modify code in inverter.py (which I think should be out-of-scope for this MR). The implementation is based on the current Sandia-multi inverter, and uses the same efficiency calculations. The battery is treated as a separate MPP, only working at a fixed voltage. The output of the function changes as well, since it makes a lot of sense to return a DataFrame instead of a Series, indicating the energy flow better and adding the clipping losses as well, which are affected by the use of a DC-connected battery.

@mikofski
Copy link
Member

mikofski commented Nov 3, 2022

Thanks @Peque for your patient explanations! The progress on this sounds great! How close to being complete? Will there be some presentation or write up to summarize the work to the community and NumFOCUS?

@Peque
Copy link
Contributor Author

Peque commented Nov 4, 2022

@mikofski Thanks to you for your feedback an interest. 😊

I would consider this to be ready as-is since it fulfills the initial goals set for the project and seems to align well with the already-discussed expectations of keeping away the optimization algorithms and avoid touching the current inverter models (which would require further discussion).

As for NumFOCUS, I asked them and the only requirement is a very simple form (the SDG Report Back Form) that I am supposed to fill. You can have a look at the answers I plan to submit. Feel free to comment, specially in the last question, which I feel could be better answered by you (the pvlib maintainers).

As for further documentation, I think the best summary is included in the docs now. I do have some more information regarding the initial market research and meetings/discussions we had with the NREL team (regarding models and code reuse). I could share them if you want, but this are just notes. Also, some do not really apply. For example, after those meeting we decided to start with AC-connected batteries only and SAM's holistic model. The final result includes DC-connected batteries too and uses SAM's BatteryStateful, which adds more flexibility to support different period/frequency time series.

If there is interest, I could clean-up those notes to share them so that there is a document to understand "why", before the how/what (that is better described in the docs).

Also if/when this MR gets integrated, I could prepare a write up to summarize the work to the community (just in case there are further modifications before this gets integrated). 😊

@adriesse
Copy link
Member

Could you

Updates entries to docs/sphinx/source/api.rst for API changes.

so that we can preview the doc strings?

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.

Add initial storage support
6 participants