NREL non-uniform irradiance mismatch loss for bifacial modules #2046

echedey-ls · 2024-05-10T21:14:40Z

Closes Reduced order electrical mismatch model #1541
I am familiar with the contributing guidelines
Tests added
Updates entries in docs/sphinx/source/reference 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 (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

Adds the model described in #1541, also in #2045, NREL source.

Doc links

I expect to extend the gallery example for a day's RMAD profiling.

Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>

kandersolar

When we are happy with this PR, let's ask the model authors to review as well (they are on github). Just making a note now so that we do not forget.

pvlib/pvsystem.py

docs/examples/bifacial/plot_irradiance_nonuniformity_loss.py

pvlib/pvsystem.py

docs/examples/bifacial/plot_irradiance_nonuniformity_loss.py

Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>

echedey-ls · 2024-05-18T19:26:29Z

Thanks @kandersolar for the feedback.

I'm going to read the published paper well - most of the upgradeable parts are due to me using the preprint - and I will come up with many changes to the current PR. I plan on selecting the polynomial model from a string and optionally by specifying its coefficients.

Pending second section of example, and checking docs Co-Authored-By: César Domínguez <48208196+cesardd@users.noreply.github.com> Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>

Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>

kandersolar

Sorry to keep complaining about percentages vs unitless...

The default coefficient values (0.142, 0.032) are intended to be used with RMAD in %, but we're using them with RMAD [-], so currently the function is calculating incorrect values. To fix it, we need to make the coefficients compatible with the RMAD input. That means we need to do one of the following:

Take RMAD input in [%]
Have the function take coefficients that are applicable to RMAD [-] (and rescale the default coefficients accordingly)
Take RMAD input [-] but translate it to [%] before applying the polynomial

There are pros and cons to each. (1) is confusing, since most pvlib functions take unitless values, not percent. (2) makes the function a little more disconnected from the reference. (3) is confusing since the inputs will be a mix of unitless and %.

I think I favor option 2. But if we go that way, it will need some explanation in the docstring explaining the difference, and how to input suitable coefficient values. @echedey-ls what do you think?

Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>

echedey-ls · 2024-06-28T22:51:32Z

No worries @kandersolar , I overlooked that. In my mind it all made sense, but the quadratic parameter clearly needed to be modified. I also find no. 2 to be more convenient. I've made the required changes with a deeper reviewing than the previous suggestions. I'm confident this is alright. Please have a look whenever you want.

markcampanelli

I’m wondering if the Variables and Symbols section of the RTD is due for an update w.r.t. the bifacial modeling additions to pvlib. If so, then it might be worth a followup ticket.

https://pvlib-python.readthedocs.io/en/stable/user_guide/variables_style_rules.html

echedey-ls · 2024-06-29T16:01:28Z

Thx for the review @markcampanelli !
Which variables do you miss from that table? rmad, coefficients, fill_factor[_reference]? I'd say the first one and fill_factor_reference are niche of this function. I find coefficients self-explanatory. IMO fill_factor is a good candidate.

markcampanelli · 2024-06-29T16:10:49Z

@echedey-ls TBH: I did not make a detailed review here. I am very inexperienced with bifacial modeling, and this MR seemed to introduce some new concepts, so I thought I’d see where the “newbie terminology” documentation was at w.r.t. bifacials. I don’t think I’m the best judge what to add and what to not add, and it doesn’t have to be decided in this MR. Maybe when all the various GSoC projects are wrapping up it could be reassessed more holistically?

echedey-ls · 2024-06-29T16:21:45Z

Don't worry, I'm practically self-taught on PV so you can imagine how bad my criteria is. Nevertheless there is a lot more value in what somebody without a deep understanding can acquire than an expert IMHO. I ask you, would you be able to apply this model to any modelling after reading it? Do you have any doubts regarding it? Except for the part that the backside cell-by-cell irradiance data usually lacks.

Maybe when all the various GSoC projects are wrapping up it could be reassessed more holistically?

I'm pretty sure this PR will be the last one bifacial-related among us GSoC students.

kandersolar · 2024-06-30T12:25:52Z

Variables and Symbols is very much overdue for an update! See #1253

cwhanse · 2024-06-30T15:13:13Z

IMO fill_factor is a good candidate.

Agree. rmad, not yet, should be added if another use emerges.

docs/examples/bifacial/plot_irradiance_nonuniformity_loss.py

docs/sphinx/source/whatsnew/v0.11.1.rst

pvlib/bifacial/loss_models.py

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Co-Authored-By: Cliff Hansen <5393711+cwhanse@users.noreply.github.com>

echedey-ls · 2024-06-30T16:59:24Z

Thx for the review @cwhanse

kandersolar

A couple more nitpicks but otherwise LGTM!

pvlib/bifacial/loss_models.py

Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>

kandersolar · 2024-07-01T17:30:21Z

Thanks @echedey-ls!

cdeline · 2024-07-01T17:45:49Z

Hi @kandersolar, @echedey-ls and others. Thanks for going through this, and sorry for not engaging any sooner. If you'd like to see worked examples of the mismatch function and pytests for it, please check out the bifacial_radiance.mismatch module with includes mad_fn and mismatch_fit3 which may be relevant. I think these files are current as of Nov. 2023. https://github.com/NREL/bifacial_radiance/blob/development/bifacial_radiance/mismatch.py
At the least we should compare results for consistency. I can't claim that we have it worked 100% correctly, so I'm glad for the double-check here.

cdeline · 2024-07-01T17:47:21Z

Oops, just seeing that you merged already. Sorry - you guys are too efficient. :)

kandersolar · 2024-07-01T20:36:12Z

+1 from me to do a cross-comparison, but I won't have time for it myself. Any volunteers? :)

@cdeline fyi the next pvlib release isn't scheduled for a little while, so reviews on this PR are still very welcome! We can always do a follow-up if you or @shirubana have any suggestions.

echedey-ls · 2024-07-03T13:02:52Z

I suppose I can do it once I free some time, although I prefer another pair of eyes to review it.

echedey-ls added 21 commits April 26, 2024 11:11

Preliminar implementation, tests, docs and gallery example

9c68605

I expect to extend the gallery example for a day's RMAD profiling.

Merge branch 'main' into nrel-mismatch-loss

7c17712

Update versionadded

23320db

Add example

47fa80d

docstring update

84346d2

Update v0.11.0.rst

bdb9955

who would have guessed it's always the linter?

914d009

Update plot_irradiance_nonuniformity_loss.py

29f4c66

equation fixes

03011d3

RMAD properties application typo

de1744a

does this fix the link?

7f27c9f

yet another link

2cdc95f

These links will never work, I suppose

0698839

Move some "Notes" equations to example and rearrange things there

7f27f1c

give this equation it's space

58c05bd

eq rendering fixes?

b973959

maths are not mathing today

448b647

???

5768473

??? x2

b1a7e4a

??? x3

d1b476d

i feel stupid for this

d037314

echedey-ls marked this pull request as ready for review May 13, 2024 21:47

Change preprint ref to published one

4ec952b

Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>

kandersolar added this to the 0.11.0 milestone May 17, 2024

kandersolar added the enhancement label May 17, 2024

kandersolar reviewed May 17, 2024

View reviewed changes

Published paper coeffs

b0cadb5

Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>

echedey-ls marked this pull request as draft May 18, 2024 19:27

Lots of improvements

9f51b68

Pending second section of example, and checking docs Co-Authored-By: César Domínguez <48208196+cesardd@users.noreply.github.com> Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>

echedey-ls and others added 4 commits June 28, 2024 18:36

Kevin's review (III)

436b0e2

Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>

Update v0.11.1.rst

7d91db6

Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>

minor changes

b1b51bd

Forgot to update tests

3991800

Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>

kandersolar reviewed Jun 28, 2024

View reviewed changes

echedey-ls and others added 3 commits June 29, 2024 00:41

Accordingly modify model, IO, docs to unitless model

aeaabb4

Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>

Update loss_models.py

9289412

Update loss_models.py

f025133

Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>

markcampanelli reviewed Jun 29, 2024

View reviewed changes

cwhanse approved these changes Jun 30, 2024

View reviewed changes

echedey-ls mentioned this pull request Jun 30, 2024

variables_style_rules.csv could use an update #1253

Open

echedey-ls and others added 2 commits June 30, 2024 18:53

Apply suggestions from code review

69a5acf

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Code review from Cliff

945d61a

Co-Authored-By: Cliff Hansen <5393711+cwhanse@users.noreply.github.com>

kandersolar approved these changes Jul 1, 2024

View reviewed changes

pvlib/bifacial/loss_models.py Outdated Show resolved Hide resolved

pvlib/bifacial/loss_models.py Outdated Show resolved Hide resolved

echedey-ls and others added 2 commits July 1, 2024 19:06

Rendering issues, code review from Kevin

cecc337

Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>

Little missing newline

289dbeb

kandersolar merged commit 8d172c4 into pvlib:main Jul 1, 2024
30 checks passed

echedey-ls mentioned this pull request Jul 29, 2024

NREL mismatch loss addendum #2147

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NREL non-uniform irradiance mismatch loss for bifacial modules #2046

NREL non-uniform irradiance mismatch loss for bifacial modules #2046

echedey-ls commented May 10, 2024 •

edited

Loading

kandersolar left a comment

echedey-ls commented May 18, 2024

kandersolar left a comment

echedey-ls commented Jun 28, 2024

markcampanelli left a comment

echedey-ls commented Jun 29, 2024

markcampanelli commented Jun 29, 2024

echedey-ls commented Jun 29, 2024

kandersolar commented Jun 30, 2024

cwhanse commented Jun 30, 2024

echedey-ls commented Jun 30, 2024

kandersolar left a comment

kandersolar commented Jul 1, 2024

cdeline commented Jul 1, 2024

cdeline commented Jul 1, 2024

kandersolar commented Jul 1, 2024

echedey-ls commented Jul 3, 2024

NREL non-uniform irradiance mismatch loss for bifacial modules #2046

NREL non-uniform irradiance mismatch loss for bifacial modules #2046

Conversation

echedey-ls commented May 10, 2024 • edited Loading

Doc links

kandersolar left a comment

Choose a reason for hiding this comment

echedey-ls commented May 18, 2024

kandersolar left a comment

Choose a reason for hiding this comment

echedey-ls commented Jun 28, 2024

markcampanelli left a comment

Choose a reason for hiding this comment

echedey-ls commented Jun 29, 2024

markcampanelli commented Jun 29, 2024

echedey-ls commented Jun 29, 2024

kandersolar commented Jun 30, 2024

cwhanse commented Jun 30, 2024

echedey-ls commented Jun 30, 2024

kandersolar left a comment

Choose a reason for hiding this comment

kandersolar commented Jul 1, 2024

cdeline commented Jul 1, 2024

cdeline commented Jul 1, 2024

kandersolar commented Jul 1, 2024

echedey-ls commented Jul 3, 2024

echedey-ls commented May 10, 2024 •

edited

Loading