- 
                Notifications
    You must be signed in to change notification settings 
- Fork 124
          Implement hash consing for Sym
          #658
        
          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
Conversation
4be9924    to
    9be3d03      
    Compare
  
    | Looks reasonable enough and the implementation is clean. Are there performance benchmarks to see if it's going on the right direction? | 
| Considering this matches hashes to symbolic variables, wouldn't it also need to handle hash collisions? If two  Also, if hashconsing is using a custom hash function could it also hash the metadata and not require that it be empty? | 
        
          
                src/types.jl
              
                Outdated
          
        
      | function Sym{T}(name::Symbol; metadata = NO_METADATA, kw...) where {T} | ||
| if metadata==NO_METADATA | ||
| s = Sym{T}(; name, kw...) | ||
| get!(wvd, hash2(s), s) | 
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.
We need a dictionary that takes both hash and equal to handle collision.
49a4e06    to
    8854977      
    Compare
  
    8854977    to
    c2d85c3      
    Compare
  
    28e27ff    to
    8957290      
    Compare
  
    b2afdf6    to
    765293a      
    Compare
  
    | requesting review @AayushSabharwal @ChrisRackauckas | 
This PR implements hash consing for
Symas the first step towards applying hash consing across SymbolicUtils.jl.Motivation:
Hash consing enables efficient memory usage and potential performance improvements by ensuring that only one unique copy of a symbolic expression exists in memory.
Implementation Details:
WeakValueDictis used to store existing symbolic objects. This choice ensures that symbolic objects without strong references are eligible for garbage collection.BasicSymbolicstruct is modified to bemutableto comply with the requirements ofWeakValueDict(due to how Julia implements finalizers).hash2, is introduced to incorporatesymtypeinto the hash calculation. This approach was chosen over modifying the existinghashfunction to avoid breaking many tests because ordering is dependent on hash values.isequal2function is added for metadata comparison in addition toBase.isequal, because directly modifyingBase.isequalbreaks numerous tests in ModelingToolkit.jl.Future PRs will apply hash consing to other
BasicSymbolicsubtypes.