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

Export denominator! and numerator! #1959

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Export denominator! and numerator! #1959

wants to merge 1 commit into from

Conversation

fingolfin
Copy link
Member

Also allow passing pointers into numerator!, and change both to not
use ccall. This makes it a little bit faster.

Before:

julia> using Nemo; z = ZZRingElem(); a = QQFieldElem(2, 3);

julia> @btime Nemo.numerator!($z, $a);
  5.208 ns (0 allocations: 0 bytes)

julia> @btime Nemo.denominator!($z, $a);
  5.166 ns (0 allocations: 0 bytes)

julia> a = a^100;

julia> @btime Nemo.numerator!($z, $a);
  7.083 ns (0 allocations: 0 bytes)

julia> @btime Nemo.denominator!($z, $a);
  6.750 ns (0 allocations: 0 bytes)

After:

julia> using Nemo; z = ZZRingElem(); a = QQFieldElem(2, 3);

julia> @btime numerator!($z, $a);
  4.000 ns (0 allocations: 0 bytes)

julia> @btime denominator!($z, $a);
  4.000 ns (0 allocations: 0 bytes)

julia> a = a^100;

julia> @btime numerator!($z, $a);
  5.875 ns (0 allocations: 0 bytes)

julia> @btime denominator!($z, $a);
  5.625 ns (0 allocations: 0 bytes)

Resolves #1863

This is a breaking change because older Hecke versions define their own numerator! and would be broken by this change (though newer Hecke should be fine -- we'll see).

@fingolfin
Export denominator! and numerator!
bcbb940 
Also allow passing pointers into `numerator!`, and change both to not
use `ccall`. This makes it a little bit faster.

Before:

    julia> using Nemo; z = ZZRingElem(); a = QQFieldElem(2, 3);

    julia> @Btime Nemo.numerator!($z, $a);
      5.208 ns (0 allocations: 0 bytes)

    julia> @Btime Nemo.denominator!($z, $a);
      5.166 ns (0 allocations: 0 bytes)

    julia> a = a^100;

    julia> @Btime Nemo.numerator!($z, $a);
      7.083 ns (0 allocations: 0 bytes)

    julia> @Btime Nemo.denominator!($z, $a);
      6.750 ns (0 allocations: 0 bytes)

After:

    julia> using Nemo; z = ZZRingElem(); a = QQFieldElem(2, 3);

    julia> @Btime numerator!($z, $a);
      4.000 ns (0 allocations: 0 bytes)

    julia> @Btime denominator!($z, $a);
      4.000 ns (0 allocations: 0 bytes)

    julia> a = a^100;

    julia> @Btime numerator!($z, $a);
      5.875 ns (0 allocations: 0 bytes)

    julia> @Btime denominator!($z, $a);
      5.625 ns (0 allocations: 0 bytes)
@codecov Codecov
Copy link

codecov bot commented Dec 4, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.94%. Comparing base (00e302b) to head (bcbb940).
Report is 2 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #1959   +/-   ##
=======================================
  Coverage   87.93%   87.94%           
=======================================
  Files          99       99           
  Lines       36416    36428   +12     
=======================================
+ Hits        32024    32036   +12     
  Misses       4392     4392

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lgoettgens lgoettgens Awaiting requested review from lgoettgens

@thofma thofma Awaiting requested review from thofma

Assignees
No one assigned
Labels
breaking
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Import numerator! denominator! from src/QuadForm/Enumeration.jl in Hecke.jl
1 participant
@fingolfin