Use circstd**2 for variance to ensure consistency across SciPy versions #50
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.
Problem
In SciPy >=1.8, the
circvar
function was changed to return a normalized variance (bounded [0,1]), whereas older versions returned an unnormalized variance. This breaks the_sum_var()
calculation in the repository, because bond (linear) variances are still absolute, and summing them with normalized angular variances no longer makes sense.How to reproduce
Installing different SciPy versions
Testing script
The following script shows the difference in behavior between SciPy <1.8
and SciPy >=1.8:
Running script
Results
Solution
This PR updates the
VectorData.analyze()
method to calculate variance ascircstd**2
for periodic data:And similarly for linear data for consistency.
This ensures
_sum_var()
produces the expected combined variance consistently across all supported SciPy versions.Related issue
Fixes #41
The terms in
_sum_var()
(for bonds and angles) should ideally be in the same units. Currently, it seems that angles are in degrees rather than radians. Not sure if this is intentional. Therefore an extra conversion step might be needed.