Improve performance of NormalizerViaRadical #2924
Conversation
hulpke
commented
Oct 16, 2018
hulpke
commented
- Avoid unnecessary membership test that would be expensive
- Clean up commented-out debugging code
- Improve action on cohomology by going to factor modules first
lib/norad.gi
Outdated
| @@ -562,6 +552,7 @@ local sus,ser,len,factorhom,uf,n,d,up,mran,nran,mpcgs,pcgs,pcisom,nf,ng,np,sub, | |||
| # work in quotient modules first | |||
| module:=GModuleByMats(Concatenation(ngm,npm),f); | |||
| sumos:=Reversed(MTX.BasesCompositionSeries(module)); | |||
| Info(InfoFitFree,2,"module layers ",List(sumos,Length)); | |||
There was a problem hiding this comment.
nitpick (can be ignored): odd indentation (perhaps spaces vs tabs?)
| ranges:=List([1..Length(part)], | ||
| x->[(x-1)*Length(lfamo)+1..x*Length(lfamo)]); | ||
|
|
||
| sub:=Concatenation(List(part,a->List(lfamo,x->Zero(f)))); |
There was a problem hiding this comment.
Isn't this just creating a matrix equal to NullMat(Length(part), Length(lfamo), f)?
There was a problem hiding this comment.
No. It is ListWithIdenticalEntries(Length(part)*Length(lfamo),Zero(f)). The Concatenation makes it clear that this is cohomology liosted as module entries associated to the generators in part.
There was a problem hiding this comment.
Indeed, somehow I (rather stupidly) missed the Concatenation bit.
lib/norad.gi
Outdated
| l:=Concatenation(l); | ||
| l:=SiftedVector(bound,l); | ||
| ConvertToVectorRep(l,Size(f)); | ||
| MakeImmutable(l); |
There was a problem hiding this comment.
you could also replace these two lines by l := ImmutableVector(f, l);, which may have advantages as the work on MatrixObj proceeds (but you can also leave it, and we'll convert this then).
There was a problem hiding this comment.
Done. (Code predated ImmutableVector and the change here was indent or similar.)
lib/norad.gi
Outdated
| return l; | ||
| end; | ||
|
|
||
| # as corrections involve only pcgs parts, the images inthe |
Codecov Report
@@ Coverage Diff @@
## master #2924 +/- ##
==========================================
+ Coverage 78.9% 78.91% +<.01%
==========================================
Files 681 681
Lines 346670 346684 +14
==========================================
+ Hits 273555 273576 +21
+ Misses 73115 73108 -7
|
within NormalizerViaRadical in matrix groups.
…uce orbit length.
PaulaHaehndel
added
kind: enhancement
fingolfin
changed the title
fingolfin
added
release notes: added
