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 dummy hash functions for arb types #1954

Merged
merged 1 commit into from
Dec 5, 2024

Conversation

lgoettgens
Copy link
Collaborator

Some progress towards oscar-system/Oscar.jl#2222, actually resolves all Nemo cases listed there.

These hash functions are valid, but not good. Once we have a way to safely access the mid and rad, these can be improved by delegating to hash functions for arf or mag (which then need someone to look at flint implementations).

Copy link

codecov bot commented Nov 29, 2024

Codecov Report

Attention: Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Project coverage is 87.88%. Comparing base (73e3de8) to head (1e7251a).
Report is 11 commits behind head on master.

Files with missing lines Patch % Lines
src/arb/Complex.jl 0.00% 2 Missing ⚠️
src/arb/Real.jl 0.00% 2 Missing ⚠️
src/arb/acb.jl 0.00% 2 Missing ⚠️
src/arb/arb.jl 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1954      +/-   ##
==========================================
- Coverage   87.93%   87.88%   -0.05%     
==========================================
  Files          99       99              
  Lines       36402    36424      +22     
==========================================
+ Hits        32010    32013       +3     
- Misses       4392     4411      +19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fingolfin
Copy link
Member

Perhaps @albinahlback or @fredrik-johansson have suggestions for what might be "good" (or at least not completely horrible) hash functions for arb types?

@albinahlback
Copy link
Contributor

They should fit inside a UInt? If so, I suppose concatenating trailing bits of mantissa and exponent would not be a terrible idea? Perhaps also fit the sign bit in some way.

@albinahlback
Copy link
Contributor

albinahlback commented Dec 3, 2024

I believe fmpz already have a straight forward hash function (it just takes the first limb) that you should be able to use, but I cannot seem to find it at the moment.

Edit: Found it. It is calcium_fmpz_hash, and is defined in calcium.h.

Edit2: You can utilize this to access the exponent, I meant to say.

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

Fine by me. But perhaps also open an issue to remind us we need to improve these??

@thofma thofma merged commit 2d12643 into Nemocas:master Dec 5, 2024
22 of 24 checks passed
@lgoettgens lgoettgens deleted the lg/arb-hash branch December 9, 2024 14:46
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.

4 participants