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

Improve Symbolic Object Hashing to Include Metadata and Symtype #670

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

bowenszhu
Copy link
Member

@bowenszhu bowenszhu commented Nov 5, 2024

This pull request enhances the hashing of symbolic objects to incorporate metadata and symtype. This addresses potential hash collisions between mathematically equivalent expressions with differing metadata or symtypes, which helps #658.

To maintain compatibility with the existing ordering of symbolic objects, which relies on the original hash function, the original implementation has been preserved as hash2.

The data structures for Add and Mul utilize Dict. However, because Dict internally uses Base.hash for key placement, this change introduces some ordering differences, leading to failures in several existing tests. Further investigation and potential mitigation strategies for these test failures are needed as mentioned in #667. This PR opens the discussion for how to best address these ordering issues while benefiting from the improved hash incorporating metadata and symtype.

@bowenszhu bowenszhu marked this pull request as draft November 7, 2024 15:56
@jpfairbanks
Copy link
Contributor

jpfairbanks commented Nov 8, 2024

Will this also change equality to include metadata and symtype? If you have two variables with the same name, but different metadata, those should probably be nonequal.

Edit: This is actually #669

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants