Repository navigation
scaling.wvm: return unchanged signal for a single site - #2896
Conversation
With one position, pdist returns an empty distance array and the effective-size optimizer converges to n_dist=0, so _compute_vr divided by zero and scaling.wvm returned an all-NaN signal. A single site has no variability reduction, so _compute_vr now returns a reduction factor of 1 for every timescale and wvm passes the input through unchanged. Fixes pvlib#2895 Signed-off-by: bfs2021 <101915083+bfs2021@users.noreply.github.com>
Hey @bfs2021! 🎉Thanks for opening your first pull request! We appreciate your If AI is used for any portion of this PR, you must vet the content Finally, be sure the PR description includes the PR |
Signed-off-by: bfs2021 <101915083+bfs2021@users.noreply.github.com>
kandersolar
left a comment
There was a problem hiding this comment.
I haven't checked the bug fix yet, but at least the what's new entry can be edited
| * :py:func:`pvlib.tracking.calc_surface_orientation` returns scalar when input | ||
| is scalar (:issue:`2881`, :pull:`2882`) | ||
| * :py:func:`pvlib.scaling.wvm` no longer returns all NaNs when ``positions`` | ||
| contains a single site (:issue:`2895`, :pull:`2896`) by :ghuser:`bfs2021` |
There was a problem hiding this comment.
| contains a single site (:issue:`2895`, :pull:`2896`) by :ghuser:`bfs2021` | |
| contains a single site (:issue:`2895`, :pull:`2896`) |
Move the contributor credit to the Contributors section per the what's-new convention, and list bfs2021 there. Signed-off-by: bfs2021 <101915083+bfs2021@users.noreply.github.com>
|
Both whatsnew suggestions applied (fe02197) — the credit moved to the Contributors section and the bullet now ends with the issue/pull references. On the Note: the run also confirms the "If AI is used for any portion of this PR, you must vet the content" note — this PR was developed with AI assistance and I've vetted the change and its tests. |
cwhanse
left a comment
There was a problem hiding this comment.
I think we should use this PR to clarify the positions input. Although the current code parses a list (or array) of lists, e.g. positions=[[x1, y1], [x2, y2], ...], that ought not to be encouraged, since the downstream function scipy.distance.pdist` is looking for a 2d array of shape (N, ndim) where ndim is the number of coordinates for each point (2 in our case). The comment is also a little unclear regarding what a position means.
- positions is documented as an (N, 2) array of (x, y) coordinates in both wvm and _compute_vr, which is the shape pdist consumes. - The single-site guard is len(pos) == 1 now: an empty array is out of scope and falls through to produce an invalid result. Signed-off-by: bfs2021 <101915083+bfs2021@users.noreply.github.com>
|
Thanks for the review — applied in 8f64f86:
|
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
|
thanks @bfs2021 |
Problem
pvlib.scaling.wvmreturns an entirely NaN output for a layout with one coordinate pair. See #2895 for the full report, including the float-precision analysis (0.29 * 100 == 28.999...-style rounding is a separate symptom of the samerounder).Root cause
With a single position,
scipy.spatial.distance.pdistreturns an empty distance array, son_pairs = 0and the effective-size objective has two roots (x=0 and x=1). The optimizer converges ton_dist = 0, which makes the Eq. 6 denominator2 * sum(rho) + n_distequal to zero — dividing by zero yields NaN for every timescale, and the reconstructed signal is all NaN.Fix
_compute_vrnow returns a variability reduction factor of 1 for every timescale whenpositionscontains a single site. Per the WVM equations, one site has no pairwise correlations and therefore no variability reduction, sowvmpasses the input signal through unchanged — the behavior the issue describes as expected.Testing
test_compute_vr_single_site:_compute_vrwith one position returns all ones for every timescale.test_wvm_single_site:wvmwith one position returns the input signal unchanged (previously all NaNs).pytest tests/test_scaling.py→ 18 passed (16 existing + 2 new), Python 3.12, Windows.Fixes #2895
Checklist
docs/sphinx/source/whatsnew/v0.16.2.rst(:issue:2895, :pull:2896, by :ghuser:bfs2021)