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

ToricVarieties: Add Base.hash #1264

Closed
HereAround opened this issue Apr 17, 2022 · 2 comments · Fixed by #2373
Closed

ToricVarieties: Add Base.hash #1264

HereAround opened this issue Apr 17, 2022 · 2 comments · Fixed by #2373

Comments

@HereAround
Copy link
Member

Should we add Base.hash for our custom types (AffineNormalToricVarieties, AbstractNormalToricVarieties, ToricDivisors, ToricDivsiorClasses, ToricLineBundles, (soon ToricVectorBundles and ToricCoherentSheaves) and many more...)?

C.f.: https://docs.julialang.org/en/v1/base/base/#Base.hash

@thofma
Copy link
Collaborator

thofma commented Apr 18, 2022

Here is my opinion.

  1. Element types should definitely define a two argument Base.hash method so that x == y implies hash(x, h) == hash(x, y). This leads to less surprises down the road with Dict, Set or unique. It gets more complicated if there is no canonical way to represent elements, but where one can still decide x == y. Then it is best to define hash(x, h) == zero(UInt) for this type.
  2. For parent types it is a bit more complicated, since the meaning of == is not too clear. For almost all the parent types coming from AbstractAlgebra/Nemo/Hecke/Singular, we consider parent objects equal if they are identical (so R == S always falls back to R === S, which is equivalent to objectid(R) == objectid(S), i.e., the "addresses" in memory are equal.). As the default hash for mutable types falls back to comparing objectid's, this is fine. So for example,
    julia> R = abelian_group([2])
    GrpAb: Z/2
    
    julia> S = abelian_group([2])
    GrpAb: Z/2
    
    julia> R == S
    false
    
    julia> R === S
    false
    
    julia> hash(R)
    0x9ae54f16dff512cb
    
    julia> hash(S)
    0xdfa4d36923bf8c61

Someone (@fingolfin, @ThomasBreuer) can probably say something more about groups.

@fingolfin
Copy link
Member

We are currently more generous for == when it comes to permutation groups, mostly because we are following GAP semantics. For fp groups it's basically the same as for the ablian groups above.

I do see a method

function Base.:(==)(td1::ToricDivisor, td2::ToricDivisor)
    return toric_variety(td1) === toric_variety(td2) && coefficients(td1) == coefficients(td2)
end

so that means there definitely should be a Bash.hash method for ToricDivisor

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

Successfully merging a pull request may close this issue.

4 participants