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

Introduce kernel-level LCM_INT #2019

Merged
merged 2 commits into from
Dec 17, 2017

Conversation

markuspf
Copy link
Member

This is done using the gmp function mpz_lcm. For large integers this leads to some speedup (over the GAP variant) when computing least common multiples.

I am not entirely sure whether this is worth it, but I open the pull request just in case. The background is that I compute the LCM of a very long list of fairly large integers.

A further pull request (which has a much bigger impact for my examples) will provide a divide-and-conquer folding function for lists.

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.

Dunno whether it is "worth it", but it doesn't hurt either, so why not :-).

src/integer.c Outdated
@@ -2202,6 +2202,43 @@ Obj FuncGCD_INT ( Obj self, Obj opL, Obj opR )
return GcdInt( opL, opR );
}

Obj LcmInt ( Obj opL, Obj opR )
Copy link
Member

Choose a reason for hiding this comment

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

I recently started using clang-format for new entries in this file (and a few older ones). Perhaps use this here, too? I.e. just do git clang-format HEAD^ && git commit --amend -C HEAD

@codecov
Copy link

codecov bot commented Dec 12, 2017

Codecov Report

Merging #2019 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2019      +/-   ##
==========================================
+ Coverage      66%   66.02%   +0.01%     
==========================================
  Files         898      898              
  Lines      273275   273285      +10     
  Branches    12745    12773      +28     
==========================================
+ Hits       180384   180424      +40     
+ Misses      90069    90038      -31     
- Partials     2822     2823       +1
Impacted Files Coverage Δ
lib/integer.gi 78.12% <ø> (+0.02%) ⬆️
hpcgap/lib/integer.gi 65.87% <ø> (-0.06%) ⬇️
src/integer.c 88.28% <100%> (+0.2%) ⬆️
src/hpc/traverse.c 77.99% <0%> (-0.39%) ⬇️
hpcgap/lib/hpc/stdtasks.g 38.61% <0%> (-0.26%) ⬇️
src/stats.c 79.43% <0%> (-0.14%) ⬇️
src/hpc/threadapi.c 34.55% <0%> (-0.1%) ⬇️
src/funcs.c 76.37% <0%> (+0.13%) ⬆️
extern/gmp/mpz/divexact.c 84.21% <0%> (+15.78%) ⬆️
extern/gmp/mpz/lcm.c 93.33% <0%> (+93.33%) ⬆️

@markuspf markuspf force-pushed the add-lcm-kernel branch 3 times, most recently from ba3cff6 to 64f783c Compare December 12, 2017 16:26
@fingolfin
Copy link
Member

BTW, we don't seem to have any test cases for Lcm (let alone LcmInt). If you feel like adding a few (perhaps next to the GcdInt test cases, possibly by duplicating and adjusting those?), that'd be great. If not, well, then not :-(

@ChrisJefferson ChrisJefferson merged commit 531edb4 into gap-system:master Dec 17, 2017
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.9.0 milestone Jan 22, 2018
@olexandr-konovalov olexandr-konovalov added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Jan 22, 2018
@markuspf markuspf deleted the add-lcm-kernel branch February 19, 2018 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: added PRs introducing changes that have since been mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants