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

Hashing is wrongly implemented for various ideals #4143

Open
HechtiDerLachs opened this issue Sep 24, 2024 · 5 comments
Open

Hashing is wrongly implemented for various ideals #4143

HechtiDerLachs opened this issue Sep 24, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@HechtiDerLachs
Copy link
Collaborator

HechtiDerLachs commented Sep 24, 2024

Try the following:

R, (x, y) = QQ[:x, :y]
I1 = ideal(R, x)
I2 = ideal(R, y)
I1 == I2 # returns true as it should
hash(I1) == hash(I2) # returns false!

According to the convention for hashes, objects which are == must also have the same hash. The failure of this probably makes unique! in lists of ideals fail:

length(unique!([I1, I2])) # result is 2, not 1!

Edit: I'm not sure this can be resolved in general. For ideals over polynomial rings we could hash a standard basis for the default_ordering of the base_ring. But for other rings like the localizations?

@HechtiDerLachs HechtiDerLachs added the bug Something isn't working label Sep 24, 2024
@lgoettgens
Copy link
Member

x-ref #2222

@fingolfin
Copy link
Member

There is simply no hash method for these ideals, hence the generic one from Base is used, which only is valid of == is implemented via ===.

So "someone" needs to define better (and fast!) hash methods for MPolyIdeal. I can think of three options:

  1. let hash just return zero. Of course then using these as dictionary keys becomes atrocious.
  2. let hash raise an error
  3. compute a hash over some canonical basis or so -- but computing those is expensive; even if we cache them I am not sure this is desirable??

@thofma
Copy link
Collaborator

thofma commented Sep 25, 2024

The problem is that there are methods in Base which use hashing without explicitly documenting it. Since this is an implementation detail, there might even be methods which start using hashing / dictionaries in the future. The most prominent culprit is unique/unique!.

I don't fully understand the "atrocious" part. For example, implementing unique(x) without hashing will have the same number of == (or isequal) as an approach via a dictionary where all elements hash to zero.

@HechtiDerLachs
Copy link
Collaborator Author

Just as an aside: Implementing a dummy hash for multivariate polynomial ideals in #4142 did not yet seem to remove the failure of unique for me. But other than that, I agree with @thofma .

@joschmitt
Copy link
Member

I'm late to the party, but when I put the ideals from your example in OSCAR I get false which seems correct to me.

julia> R, (x, y) = QQ[:x, :y]
(Multivariate polynomial ring in 2 variables over QQ, QQMPolyRingElem[x, y])

julia> I1 = ideal(R, x)
Ideal generated by
  x

julia> I2 = ideal(R, y)
Ideal generated by
  y

julia> I1 == I2
false

Did you mean to do some kind of quotient construction?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants