FIX: Align ordering with Magma for orders 3^7,5^7,7^7,11^7 #46
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #46 +/- ##
=========================================
Coverage ? 99.79%
=========================================
Files ? 491
Lines ? 324745
Branches ? 0
=========================================
Hits ? 324075
Misses ? 670
Partials ? 0
|
olexandr-konovalov
requested review from
markuspf,
fingolfin and
olexandr-konovalov
This was referenced Sep 14, 2019
wilfwilson
reviewed
Sep 18, 2019
fingolfin
approved these changes
Sep 20, 2019
doc/overview.xml
Outdated
|
|
||
| <P> | ||
| As of version 1.4 of this library, the arrangement of groups is the same as | ||
| in Magma, Version 2.23. In earlier release the ordering in degrees |
There was a problem hiding this comment.
Suggested change
| in Magma, Version 2.23. In earlier release the ordering in degrees | |
| in Magma, Version 2.23. In earlier release the ordering for the orders |
There was a problem hiding this comment.
Just noticed there’s something wrong in the language. “Earlier releases” maybe? (Although that implies ALL earlier releases; otherwise perhaps “some earlier releases”?) Or specifically list the releases?
There was a problem hiding this comment.
All earlier releases is indeed what is meant. So we should indeed write "In earlier releases" or even "In all earlier releases of this library" (to avoid ambiguity with releases of Magma).
hulpke
force-pushed
the
master
branch
2 times, most recently
from
39ecd94 to
5b45acd
Compare
The permutations applied had been wrongly implemented and were based on wrong data. New permutations, based on explicitly comparing with Magma data were used to determine correct permutations. The test for the permutations in `ordering.tst` was changed, as the old test relied on the permutations having small support, which is not true any longer. This will fix gap-packages#38 Removed the old tests as they required that the permutations were of small support. (They also were wrong) Fixed hashmark and added tests Added documentation for the new ordering.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The permutations applied had been wrongly implemented and were based on
wrong data. New permutations, based on explicitly comparing with Magma data
were used to determine correct permutations.
The test for the permutations in
ordering.tstwas changed, as the old testrelied on the permutations having small support, which is not true any
longer.
This will fix #38
Removed the old tests as they required that the permutations were of small
support. (They also were wrong)
Version number change. Fixed hashmark and added tests
Added documentation for the new ordering and reference to Magma ordering