Add SpeedupDataPcHom - #6449
Add SpeedupDataPcHom#6449
Conversation
fingolfin
added
topic: performance
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6449 +/- ##
==========================================
- Coverage 79.01% 78.99% -0.02%
==========================================
Files 685 646 -39
Lines 293519 279134 -14385
Branches 8667 7904 -763
==========================================
- Hits 231924 220502 -11422
+ Misses 59794 57112 -2682
+ Partials 1801 1520 -281 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Implements a chunked caching scheme for evaluating bijective GroupGeneralMappingByPcgs homomorphisms on PC words. Splits the source pcgs into chunks, caching linear combinations of generator images per exponent pattern, with the bottom elementary abelian layer handled via a matrix over the corresponding prime field. Includes a corresponding ImagesRepresentative method that uses this cached data for a >2.5x speedup, per the included GAPDoc example.
This excludes timing factor, as test overhead kills it.
fingolfin
left a comment
fingolfin
left a comment
There was a problem hiding this comment.
I just resolved a merge conflict.
Overall this PR seems fine, but as long as we don't the functionality for the public, it is of limited use. Given that you took the effort to write a GAPDoc comment, shouldn't we hook it up into the reference manual?
Thank you! This is intended as a first step towards including hybrid groups. If that is the case it will be used implicitly.
The documentation is in part thanks to Claude :-| . (I know the value of documentation but always give up with the GAPdoc format ...) |
fingolfin
left a comment
fingolfin
left a comment
There was a problem hiding this comment.
The code seems sensible. I'd slightly improve the test, and perhaps some comments can be clarified etc.
| if r=fail then TryNextMethod();fi; | ||
| rg:=r.groupData; | ||
| depths:=rg.depths; | ||
| # in next line subtract 1 to use the lowest level matrix |
There was a problem hiding this comment.
I don't understand. The next line does not subtraction. Is the comment outdated?
There was a problem hiding this comment.
While I still am curious about this comment, I don't mean to hold this PR up over it.
Co-authored-by: Max Horn <max@quendi.de>
|
@hulpke If you have no objections I'll apply my suggestions and merge the PR afterwards. |
No objection. I thought you were expecteing an explanation of the comment in line 839 first, which was holding me back. PS. And I'm happy to resolve the merge conflicts that will arise b/c of the automorphism group updates. |
Co-authored-by: Max Horn <max@quendi.de>
Implements a chunked caching scheme for evaluating bijective GroupGeneralMappingByPcgs homomorphisms on PC words. Splits the source pcgs into chunks, caching linear combinations of generator images per exponent pattern, with the bottom elementary abelian layer handled via a matrix over the corresponding prime field. Includes a corresponding ImagesRepresentative method that uses this cached data for a >2.5x speedup, per the included GAPDoc example.
2 participants
Implements a chunked caching scheme for evaluating bijective GroupGeneralMappingByPcgs homomorphisms on PC words. Splits the source pcgs into chunks, caching linear combinations of generator images per exponent pattern, with the bottom elementary abelian layer handled via a matrix over the corresponding prime field. Includes a corresponding ImagesRepresentative method that uses this cached data for a >2.5x speedup, per the included GAPDoc example.