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.

Sign up for GitHub

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

Jump to bottom

add trivial hash function for spoly #121

Closed
wants to merge 6 commits into from
Closed

add trivial hash function for spoly #121

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0f99e56
add trivial hash function for spoly
heiderich Jun 27, 2019
9f7229e
add test for unique for arrays of spolys
heiderich Jun 27, 2019
754f514
add UInt parameter for hash function
heiderich Jun 28, 2019
1bb108f
add function test_spoly_hash()
heiderich Jun 28, 2019
45400e3
use total_degree in Base.hash
heiderich Jun 30, 2019
2a927c5
replace product by sum in hash function
heiderich Jul 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/poly/poly.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,17 @@ function Base.iterate(x::Nemo.Generic.MPolyMonomials{spoly{T}}, state) where T <
end
end

###############################################################################
#
# Hash functions
#
###############################################################################

function Base.hash(p::spoly{T}, h::UInt) where T <: Nemo.RingElem
return total_degree(p) * h # FIXME: Implement a more useful hash function instead
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since Base.hash by default sets h=UInt(0), it's not really much better than just h ;-)

I have no idea what are the inners of spoly, but a simple implementation could be
hash(p::spoly{T}, h::UInt) where T = xor(hash(spoly), hash(coefficients_or_something_unique_to(p), h))
hash(spoly) can be of course precomputed first (add a comment).
Or if You can iterate over coefficients of p then

hash(p::spoly, h::UInt) = xor(HASH_SPOLY, reduce(xor, hash(c, h) for c in coeffs(p))

But I'd really like to see it as UInt(0) globally.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't make the hash depend on the object pointer at all. You have to use the data in the actual polynomial, i.e. the coefficients.

For about the fifth time, this is not difficult to do in the slightest and we have examples of how to do it in Nemo and AbstractAlgebra.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kalmarek I hope the version in 2a927c5 is a little better.

end


###############################################################################
#
# String I/O
Expand Down
24 changes: 24 additions & 0 deletions test/poly/spoly-test.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,28 @@ function test_spoly_differential()
@test jf == x^3
end

function test_spoly_unique()
print("spoly.unique...")

R, (x, y) = PolynomialRing(QQ, ["x", "y"])

@test unique([x, x+y-y]) == [x]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test will occasionally pass even without overloaded hash function;
we should add test which directly compares hashes
in this case these should be the same (and we don't have negative example due to trivial definition above)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See 1bb108f.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is test_spoly_unique a useful test at all? It only tests one particular symptom, not the core issue, which is tested in test_spoly_hash. Is there another reason to test unique on polynomials like this?


println("PASS")
end

function test_spoly_hash()
print("spoly.hash...")

R, (x, y) = PolynomialRing(QQ, ["x", "y"])

@test hash(x) == hash(x+y-y)
@test hash(x,zero(UInt)) == hash(x+y-y,zero(UInt))
@test hash(x,one(UInt)) == hash(x+y-y,one(UInt))

println("PASS")
end

function test_spoly()
test_spoly_constructors()
test_spoly_printing()
Expand All @@ -475,6 +497,8 @@ function test_spoly()
test_spoly_Polynomials()
# test_convert_between_MPoly_and_SingularPoly()
test_spoly_differential()
test_spoly_unique()
test_spoly_hash()

println("")
end
Expand Down