Skip to content

Change accuracy of pvsyst_cell test function #2080

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 10, 2024

Conversation

IoannisSifnaios
Copy link
Contributor

I noticed that for the test_pvsyst_cell_ndarray, the tolerance for the test is 3, which I think is too high. Thus, I changed it to the default value.

@AdamRJensen AdamRJensen added bug GSoC Contributions related to Google Summer of Code. labels Jun 6, 2024
@IoannisSifnaios
Copy link
Contributor Author

@kandersolar, all the tests for sapm (module and cell) have a tolerance of 3. Is this correct? Or am I misunderstanding something?

image

@kandersolar
Copy link
Member

I think you are correct that these tests are in error. Looking back through history, I think this was an oversight from when we switched from the nose test framework to pytest in #204. From what I can tell, nose interprets the third parameter as the number of decimal places to check, so 3 would mean "to three decimal places", whereas pytest interprets it as the precision, so 1e-3 means "to three decimal places". It looks like #204 just copied the 3 over for those tests: https://github.com/pvlib/pvlib-python/pull/204/files#diff-c00d7870b1b61cebdc08ab0e105fff10539647591a48cccd553606cd56376055R338-R339

I suspect we may have similar issues in other test modules. Here are two ideas to identify any, if they exist:

@echedey-ls
Copy link
Contributor

@kandersolar assert_almost_equal's third parameter is the decimal which gets compared.
I suppose you were talking about assert_allclose. Fixes to the diff findings and a few more were made at #2082

FFR, regex is (assert_allclose)\((.*), ?(.*), ?([a-zA-Z0-9=]*)?(\d)\)

echedey-ls added a commit to echedey-ls/pvlib-python that referenced this pull request Jun 6, 2024
@kandersolar kandersolar added this to the 0.11.0 milestone Jun 10, 2024
Copy link
Member

@kandersolar kandersolar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks and good catch @IoannisSifnaios!

@kandersolar kandersolar marked this pull request as ready for review June 10, 2024 15:50
@kandersolar kandersolar changed the title Change accuracy of test function Change accuracy of pvsyst_cell test function Jun 10, 2024
@kandersolar kandersolar merged commit b147600 into pvlib:main Jun 10, 2024
30 of 31 checks passed
kandersolar pushed a commit that referenced this pull request Jun 10, 2024
* One-line assert_allclose

F: (assert_allclose)\((.*), ?(.*), ?([a-zA-Z0-9=]*)?(\d)\)

* I didn't save all before committing, yet again

* Revert test_irradiance change in precision

* Do not overlap with #2080
@IoannisSifnaios IoannisSifnaios deleted the fix_pvsyst_test branch June 12, 2024 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug GSoC Contributions related to Google Summer of Code. testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants