Add test for vf_ground_sky_2d view factor bounds#2690
Add test for vf_ground_sky_2d view factor bounds#2690Chirag3841 wants to merge 2 commits intopvlib:mainfrom
Conversation
|
@AdamRJensen I implemented a fix for the fencepost/off-by-one issue in |
|
@AdamRJensen I fixed the flake8 lint issues that were causing the remaining tests to fail. All tests pass locally now and flake8 is clean. |
|
This PR has significant problems (it does not fix the problem, it gets rid of some important memory optimizations, and the added test has nothing to do with the problem). @Chirag3841 as @AdamRJensen mentioned in the linked issue, pvlib is developed by solar energy modelers and is a difficult project for general programmers to get involved with. Unfortunately, our reviewer capacity is limited and we cannot accept PRs from people without solar experience. Therefore I am closing this PR. |
bifacial.utils.vf_ground_sky_2dinterpretsmax_rowsasymmetrically #1867docs/sphinx/source/referencefor API changes.docs/sphinx/source/whatsnewfor 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`).remote-data) and Milestone are assigned to the Pull Request and linked Issue.Description
Fixes an off-by-one / fencepost issue in
vf_ground_sky_2dwhere one fewer skywedge was considered on the right side compared to the left side in symmetric
geometries.
The fix includes horizon bounds (cos(0)=1 and cos(pi)=-1) so the wedge
calculation is consistent on both sides.
Tests
Added a regression test
test_vf_ground_sky_2d_returns_valid_rangeto ensurethe returned view factor stays within the physically valid range [0, 1].