Skip to content

Stop ModelChain from filling missing DC/AC values with zero (gh-1409)#2822

Closed
adi-IL wants to merge 1 commit into
pvlib:mainfrom
adi-IL:fix/modelchain-nan-in-nan-out
Closed

Stop ModelChain from filling missing DC/AC values with zero (gh-1409)#2822
adi-IL wants to merge 1 commit into
pvlib:mainfrom
adi-IL:fix/modelchain-nan-in-nan-out

Conversation

@adi-IL

@adi-IL adi-IL commented Jul 12, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes a long standing behavior where ModelChain silently replaced missing (NaN) values in its DC and AC results with zero.

Background

The single diode path ended with

self.results.dc = tuple(dc.fillna(0) for dc in self.results.dc)

and the pvwatts inverter did

self.results.ac = ac.fillna(0)

A user who passed genuine gaps (NaN) into the weather therefore received zero power for those time steps instead of NaN, masking the missing data. This is inconsistent with pvlib's documented NaN in, NaN out policy. (See gh-1409.)

The masking was originally added so that ModelChain.results.dc.sum() returned a value at night. We checked that this is no longer needed: at night the single diode model already returns zero power on its own (zero photocurrent gives zero p_mp), so removing the fill leaves nighttime correct while letting true gaps stay NaN.

The fix

Remove both fillna(0) calls. Genuine missing inputs now propagate as NaN through the DC and AC results, and nighttime remains zero power.

Testing

  • Added test_run_model_preserves_nan_inputs: feeds a three step weather series (day, NaN gap, night) through the cec/single diode DC model and asserts the gap stays NaN, night is zero, and daytime is positive.
  • Added test_pvwatts_inverter_preserves_nan_inputs: same idea through the pvwatts inverter.
  • Full tests/test_modelchain.py suite passes (159 tests).

Closes #1409

…1409)

ModelChain previously called fillna(0) on the single diode DC results and on the pvwatts AC result. This silently turned genuine missing (NaN) inputs into zero power, which is inconsistent with pvlib's 'NaN in, NaN out' policy and hides data gaps from users.

Remove both fillna(0) calls. Nighttime already returns zero power from the single diode model (zero photocurrent), so only genuine gaps are now left as NaN. Add regression tests for the DC (cec) and pvwatts AC paths, and a whatsnew entry.
@cwhanse

cwhanse commented Jul 12, 2026

Copy link
Copy Markdown
Member

I'm closing this for three reasons, none of which have to do with the improvement that the submitter proposes:

  1. This solution has not been discussed and agreed to in the relevant Issue. The submitter's comment appears entirely written by AI.
  2. The submitter hasn't included the PR template.
  3. I am fatigued with reviewing AI-enabled submissions that usually require more time from a reviewer than the submitter puts into the PR.

Sorry to be blunt. If you'd like to contribute to pvlib (we welcome that), please discuss the proposed solution to #1409 in the relevant issue, along with evidence that your solution addresses the concern.

@cwhanse cwhanse closed this Jul 12, 2026
@adi-IL adi-IL deleted the fix/modelchain-nan-in-nan-out branch July 13, 2026 05:22
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.

DC model unexpectedly fills NaNs with 0

2 participants