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

unique polynomials #120

Closed
heiderich opened this issue Jun 27, 2019 · 11 comments · Fixed by #139
Closed

unique polynomials #120

heiderich opened this issue Jun 27, 2019 · 11 comments · Fixed by #139

Comments

@heiderich
Copy link
Contributor

Is the following expected?

julia> using Singular

Welcome to Nemo version 0.14.0

Nemo comes with absolutely no warranty whatsoever


julia> R,(x,y) = Singular.PolynomialRing(Singular.QQ, ["x","y"]);

julia> unique([x+y-y, x])
2-element Array{spoly{n_Q},1}:
 x
 x
@wbhart
Copy link
Contributor

wbhart commented Jun 27, 2019

Not really. But it is probably due to a missing hash function or something like that.

@sebasguts
Copy link
Contributor

Yep, it is indeed because of the missing hash function. You can do this by overloading Base.hash, making it always return 0 for Singular polynomials.

@wbhart
Copy link
Contributor

wbhart commented Jun 27, 2019

Better still would be to provide a working hash!

@sebasguts
Copy link
Contributor

Absolutely, yes. But as long as we do not have one, a hash that is not compatible with == is worse than no hash.

@kalmarek
Copy link

@wbhart I'd like to point out that hash is required in the ring interface;
and Singular polys are by no means the only type which does not define it.

why don't we define Base.hash(::RingElem, h::UInt) = h (or actually for all types we define!) and in the interface just ask to provide a better function?

@wbhart
Copy link
Contributor

wbhart commented Jun 27, 2019

Why don't we just add hash functions for types that don't have them?

I don't understand this desire to implement trivial hashes. They are so terribly easy to implement properly, and there are plenty of examples.

@kalmarek
Copy link

I don't understand why we should stick to the definition which (while with some merits based on performance) gives false mathematical results. And as we can see it's very easy to forget to define hash AND to find out which types don't define it.

you don't have to take my word for it; just count the number of issues referencing this one: custom hashing is too easy to accidentally break

and now there are n+1 of them ;-)

@wbhart
Copy link
Contributor

wbhart commented Jun 27, 2019

I don't want to stick with the incorrect definition. But why waste all that effort to add trivial hash functions and tests and not the extra couple of lines to do it properly?

@fieker
Copy link
Contributor

fieker commented Jun 28, 2019 via email

@kalmarek
Copy link

@wbhart adding a correct fallback does not preclude us from providing specific hashes for particular types

@fingolfin
Copy link
Member

I support @kalmarek's suggestion as well

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 a pull request may close this issue.

6 participants