-
Notifications
You must be signed in to change notification settings - Fork 357
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
Revise some spatial tests to pytest style; avoids numercial issues due to float comparisons #2657
Conversation
…precise float comparions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bug fix looks good to me, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the tests in test_SynapseCollection_distance
might be split into 3 parts:
- Using Grid
- Using Free distribution (include randomness)
- Mixed: Both variants
Each of those has the same pattern:
- Create Nodes
- Connect
- GetConnections
- Get distance
- assert
So I think we can have one function for each of the three variants mentioned above and use pytest decorators
to parametrize the function to avoid code repetition.
testsuite/pytests/test_spatial/test_SynapseCollection_distance.py
Outdated
Show resolved
Hide resolved
@heplesser: Otherwise regarding the bug, the changes look fine. If you want to consider my suggestions, I will wait and approve later, otherwise if you think that they should belong to a different PR, then I will approve immediately. |
Co-authored-by: med-ayssar <med.ayssar@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect!
The main motivation for this PR is that tests from two modules failed on Apple silicon after upgrading to macOS 13.3.1 and XCode 14.3 because some distance values returned by NEST differ in the last decimal (mantissa bit). The difference occurs with Apple Clang 14.0.3 and GCC 12.2.0; it does not occur on Intel silicon.
All comparisons (except where exact zero is expected) now use
pytest.approx()
. The tests are also restructured in PyTest form and grouped in two classes according to the network setup they require.The tests could probably be revised further, but this is a bug fix and should therefore not be delayed.