add marion's adjustment to pvwatts_dc #2569

williamhobbs · 2025-10-07T13:05:13Z

Closes Nonlinear adjustment to pvwattsv5 dc model #2566
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.

Still some work to do . Replaces #2568.

cwhanse

I like this better than a separate function.

pvlib/pvsystem.py

cwhanse · 2025-10-07T21:31:16Z

pvlib/pvsystem.py

+
+    For example, a 500 W module that produces 95 W at 200 W/m^2 (a 5% relative
+    reduction in efficiency) would have a value of `k` = 0.01.
+


Please include Equation from above with k, I'd place it here.

See my latest change. Is that what you had in mind?

Yeah, that comment was not clear. I was thinking we should copy the equation for P_DC here, and show how k modifies the power. But the equation for the adjustment is not simple (the piecewise part), so I retract that thought. If someone wants to know how the adjustment works, we've provided code and the reference.

pvlib/pvsystem.py

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

RDaxini

Nice work @williamhobbs. Just a few simple comments/suggestions on the docs from me. Only had a quick look at the python implementation, LGTM at a glance but I could have a deeper look later if required

pvlib/pvsystem.py

doi sphinx Co-authored-by: RDaxini <143435106+RDaxini@users.noreply.github.com>

Co-authored-by: RDaxini <143435106+RDaxini@users.noreply.github.com>

RDaxini · 2025-10-08T17:01:16Z

pvlib/pvsystem.py

+    # apply Marion's correction if k is anything but zero
+    if k is not None:
+        err_1 = (k * (1 - (1 - effective_irradiance / 200)**4) /
+                 (effective_irradiance / 1000))
+        err_2 = (k * (1000 - effective_irradiance) / (1000 - 200))
+
+        pdc_marion = np.where(effective_irradiance <= 200,
+                              pdc * (1 - err_1),
+                              pdc * (1 - err_2))
+
+        # "cap" Marion's correction at 1000 W/m^2
+        if cap_adjustment is True:
+            pdc_marion = np.where(effective_irradiance >= 1000,
+                                  pdc,
+                                  pdc_marion)
+
+        pdc = pdc_marion
+


@williamhobbs have you started / do you intend to write tests? The codecov check fails since this section is not covered by tests. Happy to help with that if you need.

Haven't started, but figured I would need to. Do you have suggestions on tests to add? I have pretty limited experience with the pvlib testing structure/best practices.

I'd hand-calculate half a dozen points, using k. Test for correct output with input of three types: float, array, Series. Then test with cap_adjustment=True.

Be creative, test whatever comes to mind! Generally speaking, test a range of intended and extreme conditions.

Intended:
For the if statements, check whether the indented block is executed if the condition is met (for example, if k is not None, use some example values to check whether the correction is applied).
Whether the block is executed correctly should also be checked--- if k is not None, then check example irradiance values >200 and <=200 (is the intended behaviour executed correctly in these cases?)

Extreme:
What if the user enters non-physical values, how should these be handled and are they handled in this way? e.g. negative or NaN irradiance values

Someone else might be able to offer a clearer/more succinct explanation 😅

I'd hand-calculate half a dozen points, using k. Test for correct output with input of three types: float, array, Series. Then test with cap_adjustment=True.

Good point, check whether different data types are handled appropriately too

@williamhobbs you do not have to test with k=None, that is covered by existing tests. Any new tests would be better in their own function, e.g., test_pvwatts_dc_with_k.

@williamhobbs you do not have to test with k=None, that is covered by existing tests.

Correct, my bad. I was trying to make a general point but overlooked that in the example.
See the codecov report

Any new tests would be better in their own function, e.g., test_pvwatts_dc_with_k.

Add to test_pvsystem.py (examples also visible there)

wholmgren

Thanks @williamhobbs good stuff! Happy to see this added to pvlib, less sure about the API right now.

We've been talking about changing this function name to pvwatts_dc_v5 in #1350. I think these new parameters will look quite out of place if we do go through with that. Which makes me think a separate function might be preferable.

wholmgren · 2025-10-08T18:21:20Z

pvlib/pvsystem.py

+
+    This adjustment increases relative efficiency for irradiance above 1000
+    Wm⁻², which may not be desired. An optional input, `capped_adjustment`,
+    modifies the adjustment from [2]_ to only apply below 1000 Wm⁻².


any concerns about deviating from a reference here? I'm ok with it but imagine it could be a point of contention. perhaps some more documentation clarity would help

I also wondered about deviating from the reference. It just gives the option to apply the Marion 2008 correction up to 1000 Wm-2, and stay with standard pvwatts above that, so seems like a minor deviation.

And I can definitely add more documentation. I tend to get to long-winded and was trying to fight that. I’m open to suggestions, but will also work on some additions.

wholmgren · 2025-10-08T18:26:30Z

pvlib/pvsystem.py

+                 (effective_irradiance / 1000))
+        err_2 = (k * (1000 - effective_irradiance) / (1000 - 200))
+
+        pdc_marion = np.where(effective_irradiance <= 200,


np.where will break the paradigm of "return the same object type that was input" since it always returns an array. Options:

keep np.where, cast output to match input

switch to slicing, assume array input

switch to slicing, promote scalars to arrays for compatibility

I see what you mean. I tried a few things, but can’t seem to figure out how to get your proposed solutions to work. Any pointers or examples?

pvlib/pvsystem.py

wholmgren · 2025-10-08T18:33:21Z

pvlib/pvsystem.py

           (1 + gamma_pdc * (temp_cell - temp_ref)))

+    # apply Marion's correction if k is anything but zero
+    if k is not None:


needing to use the is not None paradigm is a small reason to prefer a separate function

cwhanse · 2025-10-08T18:54:42Z

We've been talking about changing this function name to pvwatts_dc_v5 in #1350. I think these new parameters will look quite out of place if we do go through with that. Which makes me think a separate function might be preferable.

I'm not dismissing your point. But to reply, PVWatts v8 (or whatever it is called now) is a moving target that does not have much documentation. I think, if we do anything with the newer PVWatts in pvlib (once it stablizes and is documented somehow), we could name that new function "pvwatts_v8" and leave the existing functions names without the version suffix.

Co-authored-by: Will Holmgren <william.holmgren@gmail.com>

…thon into add_marion

williamhobbs · 2025-10-09T22:10:31Z

pvlib/pvsystem.py

+    For positive `k` values, and `k` is typically positive, this adjustment
+    increases relative efficiency when irradiance is above 1000 Wm⁻². This may
+    not be desired, as modules with nonlinear irradiance response often have
+    peak efficiency near 1000 Wm⁻², and it is either flat or declining at
+    higher irradiance. An optional parameter, `cap_adjustment`, can address
+    this by modifying the adjustment from [2]_ to only apply below 1000 Wm⁻².


@wholmgren, does this help with clarifying the deviation?

add marion's adjustment

05f5408

williamhobbs mentioned this pull request Oct 7, 2025

add pvwatts_dc_marion #2568

Closed

8 tasks

linting

6d7ae12

cwhanse reviewed Oct 7, 2025

View reviewed changes

williamhobbs and others added 6 commits October 7, 2025 16:56

Update pvlib/pvsystem.py

40328c8

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

Update pvlib/pvsystem.py

a9dfb28

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

Update pvlib/pvsystem.py

551e722

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

Update pvlib/pvsystem.py

73e80d3

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

Update pvlib/pvsystem.py

f25b426

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

more suggested changes

380bb58

RDaxini reviewed Oct 8, 2025

View reviewed changes

pvlib/pvsystem.py Outdated Show resolved Hide resolved

pvlib/pvsystem.py Outdated Show resolved Hide resolved

pvlib/pvsystem.py Outdated Show resolved Hide resolved

pvlib/pvsystem.py Outdated Show resolved Hide resolved

williamhobbs and others added 4 commits October 8, 2025 11:41

Update pvlib/pvsystem.py

5d31dfd

doi sphinx Co-authored-by: RDaxini <143435106+RDaxini@users.noreply.github.com>

Update pvlib/pvsystem.py

ad146b5

Co-authored-by: RDaxini <143435106+RDaxini@users.noreply.github.com>

Update pvlib/pvsystem.py

69ce5e7

Co-authored-by: RDaxini <143435106+RDaxini@users.noreply.github.com>

cleaning up docs

dc58507

kandersolar added this to the v0.13.2 milestone Oct 8, 2025

kandersolar added the enhancement label Oct 8, 2025

RDaxini reviewed Oct 8, 2025

View reviewed changes

wholmgren reviewed Oct 8, 2025

View reviewed changes

williamhobbs and others added 7 commits October 8, 2025 17:00

Update pvlib/pvsystem.py

0d8b0b5

Co-authored-by: Will Holmgren <william.holmgren@gmail.com>

prevent negative power

90b7c54

Merge branch 'add_marion' of https://github.com/williamhobbs/pvlib-py…

1297448

…thon into add_marion

fix typo in error eqn

4488e3c

fix the right typo this time...

250af5a

reorder err eqns

bd16ac2

add more detail on cap_adjustment

f3041c2

williamhobbs commented Oct 9, 2025

View reviewed changes


		For example, a 500 W module that produces 95 W at 200 W/m^2 (a 5% relative
		reduction in efficiency) would have a value of `k` = 0.01.

add marion's adjustment to pvwatts_dc #2569

Are you sure you want to change the base?

add marion's adjustment to pvwatts_dc #2569

Conversation

williamhobbs commented Oct 7, 2025 • edited by kandersolar Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cwhanse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RDaxini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RDaxini Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wholmgren left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cwhanse commented Oct 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

williamhobbs commented Oct 7, 2025 •

edited by kandersolar

Loading

RDaxini Oct 8, 2025 •

edited

Loading