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

Add hash method for RayVector #4354

Merged
merged 2 commits into from
Nov 29, 2024
Merged

Add hash method for RayVector #4354

merged 2 commits into from
Nov 29, 2024

Conversation

alexej-jordan
Copy link
Collaborator

No description provided.

@alexej-jordan
Copy link
Collaborator Author

With this notion we already have a good distinction what could be equal. The disadvantage is that calculating the sign could be too demanding for more complex coefficient fields? If so, we could utilize the efficient is_zero function: Base.hash(x::RayVector, h::UInt) = hash(iszero.(x), hash(coefficient_field(x), h))

Copy link

codecov bot commented Nov 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.34%. Comparing base (4d753d8) to head (58c5aad).
Report is 14 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4354      +/-   ##
==========================================
+ Coverage   84.31%   84.34%   +0.03%     
==========================================
  Files         649      651       +2     
  Lines       86345    86525     +180     
==========================================
+ Hits        72799    72980     +181     
+ Misses      13546    13545       -1     
Files with missing lines Coverage Δ
src/PolyhedralGeometry/iterators.jl 92.13% <100.00%> (+0.08%) ⬆️

... and 18 files with indirect coverage changes

@benlorenz
Copy link
Member

benlorenz commented Nov 28, 2024

Can you add a small test for this, maybe like this:

@test length(Set(ray_vector.([[1,2,3],[2,4,6],[-1,-2,-3]]))) == 2

@benlorenz
Copy link
Member

With this notion we already have a good distinction what could be equal. The disadvantage is that calculating the sign could be too demanding for more complex coefficient fields? If so, we could utilize the efficient is_zero function: Base.hash(x::RayVector, h::UInt) = hash(iszero.(x), hash(coefficient_field(x), h))

I think the sign is fine.

@alexej-jordan alexej-jordan marked this pull request as ready for review November 29, 2024 13:38
@benlorenz benlorenz merged commit f83a525 into master Nov 29, 2024
31 checks passed
@benlorenz benlorenz deleted the aj/rayhash branch November 29, 2024 13:39
benlorenz pushed a commit that referenced this pull request Dec 12, 2024
* added hash method for RayVector; fixed wrong test

* add isequal and corresponding test for RayVector

(cherry picked from commit f83a525)
@benlorenz benlorenz mentioned this pull request Dec 12, 2024
benlorenz added a commit that referenced this pull request Dec 13, 2024
Backports for release 1.2.2:

fix galois_group problem. #4396
Fix zero-dimensional cone in cones in PolyhedralGeometry #4336
Fix up generic characteristic method for localized rings. #4346
test/PolyhedralGeometry: remove some prefer blocks #4351
Add comment about bibtool version, minor fix in testsuite #4356
Add hash method for RayVector #4354
Fix indent in AlgebraicCycles.md #4368
Fix toric typo #4367
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 this pull request may close these issues.

2 participants