Skip to content
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

ENH: compute hash without PYTHONHASHSED #444

Merged
merged 34 commits into from
Dec 20, 2024
Merged

ENH: compute hash without PYTHONHASHSED #444

merged 34 commits into from
Dec 20, 2024

Conversation

redeboer
Copy link
Member

@redeboer redeboer commented Dec 18, 2024

Closes #443

@redeboer redeboer added the ⚙️ Enhancement Improvements and optimizations of existing features label Dec 18, 2024
@redeboer redeboer self-assigned this Dec 18, 2024
@redeboer
Copy link
Member Author

redeboer commented Dec 18, 2024

@Zeyna777 it would be good to extract a separate maintenance PR from this PR that only updates the lock files and implements the formatting and fixes by Ruff (29b2d40...ffe52cc).

git fetch origin
git checkout -b update-lock origin/main
git cherry-pick 70de299 9836f66 ffe52cc
git push origin update-lock -u

Could you create that PR? Then it's easier to review this PR after that one has been merged.

@redeboer
Copy link
Member Author

redeboer commented Dec 18, 2024

I'm currently investigating why the hashes are different on CI
https://github.com/ComPWA/ampform/actions/runs/12396403891/job/34604373028?pr=444

I tried on two different Linux machines (Almalinux 9 and WSL Ubuntu 24.04), but there the tests work. To quickly check, run:

pytest tests/sympy/test_cache.py
Commit Algorithm Failing tests
29b2d40 xxh3_64 both formalisms
5b3a507 xxh64 only canonical-helicity
4d26866 xxh128 only canonical-helicity

See xxhash.com and the xxhash package for the differences between the algorithms.

@redeboer redeboer changed the title ENH: compute hash over folded expression with xxhash library ENH: compute hash over folded expression with xxHash Dec 18, 2024
@redeboer redeboer changed the title ENH: compute hash over folded expression with xxHash ENH: compute folded expression hash with xxHash Dec 18, 2024
@redeboer
Copy link
Member Author

Okay... so the hash is different depending on whether running all tests

pytest

or the specific test separately

pytest tests/sympy/test_cache.py

@redeboer
Copy link
Member Author

redeboer commented Dec 19, 2024

So, not using a fixture fixes it. I guess the moment of creating the model while running pytest somehow influences the hash. Fixed by 8e96e69.

This is not good though... it suggests that the hash depends on the runtime of the program (?). It should only depend on the binary bytes content of the object that is being cached.

It seems that ReactionInfo itself does not have a stable hash, even if it is marked as immutable (see 7bcf84b). I suspect the hash of FrozenDict (8a9a171). We should try either MappingProxyType1 (see PEP 416) or frozendict as an alternative.

Footnotes

  1. Ah okay... this type is not pickable.

@redeboer
Copy link
Member Author

Alright, so frozendict has a stable hash with xxhash 🎉 See a1d4c94. FrozenDict does not...

@redeboer redeboer changed the title ENH: compute folded expression hash with xxHash ENH: compute hash without PYTHONHASHSED Dec 20, 2024
@redeboer redeboer marked this pull request as ready for review December 20, 2024 13:47
@redeboer redeboer added this to the 0.15.5 milestone Dec 20, 2024
@redeboer redeboer enabled auto-merge (squash) December 20, 2024 13:50
@redeboer redeboer merged commit f955d0a into main Dec 20, 2024
28 checks passed
@redeboer redeboer deleted the fix-caching branch December 20, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚙️ Enhancement Improvements and optimizations of existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve hash for caching
2 participants