Fix delta_kt_prime calculation for series with internal NaN values#2698
Open
ishaan-arora-1 wants to merge 2 commits intopvlib:mainfrom
Open
Fix delta_kt_prime calculation for series with internal NaN values#2698ishaan-arora-1 wants to merge 2 commits intopvlib:mainfrom
ishaan-arora-1 wants to merge 2 commits intopvlib:mainfrom
Conversation
The previous implementation of _delta_kt_prime_dirint only handled NaN at the very first and last positions of the series. For multi-day data with nighttime NaN gaps, the edge positions adjacent to internal NaN boundaries had their delta values incorrectly halved (the 0.5 factor was applied even when only one neighbor was valid). Replace the manual NaN-filling approach with pd.DataFrame.mean(axis=1), which naturally skips NaN values. This correctly implements both Perez eqn 2 (average of both deltas when two neighbors exist) and eqn 3 (single delta when only one neighbor exists). Fixes pvlib#1847
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
docs/sphinx/source/referencefor API changes.docs/sphinx/source/whatsnewfor all changes. Includes link to the GitHub Issue with:issue:numor 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.Problem
_delta_kt_prime_dirintincorrectly calculatesdelta_kt_primefor time series containing internal NaN values (e.g., multi-day data with nighttime gaps).The previous implementation only handled NaN at the very first and last positions of the series (lines 2034-2035). For multi-day input, nighttime NaN values create internal boundaries that were not accounted for. At these boundaries, the
fill_value=0in the.add()call caused one of the two delta terms to contribute 0, but the0.5factor was still applied — effectively halving the delta value. This violates Perez eqn 3, which specifies that when only one neighbor is available, the full (unhalved) delta should be used.Solution
Replace the manual NaN-filling logic with
pd.DataFrame.mean(axis=1), as suggested by @cwhanse:DataFrame.mean(axis=1)naturally skips NaN values, so:This is simpler, correct, and handles all edge cases including the first/last positions of the series, internal NaN gaps, and single-element series.
Testing
Added
test_delta_kt_prime_dirint_multidaywhich uses the exact scenario from the bug report (a series with leading/trailing NaN values simulating nighttime gaps) and verifies:All 121 existing irradiance tests continue to pass.