scikit-hep / scikit-hep/vector
Structural v/s symbolic equality for SymPy tests
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 99
- Forks
- 42
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 6
Description
#581 raises the question of structural (x == y) v/s symbolic (simplify(x - y) == 0 or Eq(x, y)) equality. Even though symbolic equality would be easier to maintain in the longer run, it would be better to use the structural equality wherever possible to be more strict as -
- we test (or at least, aim to test when we drop Python 3.8 support (uncompyle6 does not support Python 3.9+)) the ability of vector's compute functions to operate only on data containers using the SymPy tests - https://github.com/scikit-hep/vector/blob/main/tests/test_compute_features.py
- the SymPy tests don't fail often, so the maintainability is not that big of a problem
Originally posted by @Saransh-cpp in https://github.com/scikit-hep/vector/pull/581#pullrequestreview-2806944886
Tests failing without being mathematically wrong is bad IMO, since they will sooner or later fail with sympy updates (as is the case now from 1.13.1 -> 1.14.0). Hence I prefer symbolic comparison.
In fact it is quite similar to checking floating points by an epsilon range, since the backend computation can change, but the result is still right (wrt. the scope).
Originally posted by @APN-Pucky in https://github.com/scikit-hep/vector/issues/581#issuecomment-2863341676
Thanks! My comment behind using structural equality was because the "symbolic" nature of our compute functions will solely be tested by SymPy once Python 3.8 is removed from GH Actions; hence, we would want these tests to be strict (unless we find another way - #531). But at the same time, I don't think this strictness is actually adding anything to the test suite - the symbolic equality will test the exact same thing but in a more flexible way.
Given that we use structural equality everywhere in the tests, I have reverted to keep using that for now. However, I will open an issue to discuss this further (and possibly change all equalities in the tests in the future).
Originally posted by @Saransh-cpp in https://github.com/scikit-hep/vector/issues/581#issuecomment-2927882803
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading tests/test_compute_features.py and the linked discussion in #581 to understand where structural equality is used and why symbolic equality was proposed. The work is complete when the project has an agreed equality strategy, the relevant tests consistently follow it, and the test suite passes across the supported Python versions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- testing
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100