Shortening SimpleAlgebraInfoByData and WedderburnDecompositionInfo

[angeldelriomateos]

A simple modification of SimpleAlgebraInfoByData which use GlobalSplittingOfCyclotomicAlgebra to shorten the numerical information of some cyclotomic algebras in the output of WedderburnDecompositionInfo.

[angeldelriomateos]

If you revise the changes in main.gi, more precisely at the end of the code for SimpleAlgebraInfoByData you will find some commented lines. This is prepared for the second alternative. I encountered an issue with that alternative. I wait for your reaction on this full request before going ahead with the second alternative.

[angeldelriomateos]

Upps. I discovered that WedderburnDecompositionInfo fails for SG(480,266). I am going to investigate what is the problem. At this moment I guess the problem is in GlobalSplittingOfCyclotomicAlgebra

[angeldelriomateos]

I discovered that the issue occurs with the combination of IsCyclotomicExtension and ReducingCyclotomicAlgebra. This is independent of the Shortening of WedderburnDecompositionInfo, i.e. it affects also to GlobalSplittingOfCyclotomicAlgebra. I think that I know how to fix it but I need time to do it properly. So my next pull request will be fixing this bug

[drallenherman]

I discovered that the issue occurs with the combination of IsCyclotomicExtension and ReducingCyclotomicAlgebra. This is independent of the Shortening of WedderburnDecompositionInfo, i.e. it affects also to GlobalSplittingOfCyclotomicAlgebra. I think that I know how to fix it but I need time to do it properly. So my next pull request will be fixing this bug

When I tried to load wedderga after pulling #62 the LoadPackage failed the first time. Using GlobalSplitting in main.gi seems an issue because the command hasn't been defined yet. After I loaded a second time, it was. On SG(480,266) it seemed to give a sequencing error:

Error, Variable: 'GlobalSplittingOfCyclotomicAlgebra' must have an assigned value in return GlobalSplittingOfCyclotomicAlgebra( A ) ; at /proc/cygdrive/C/gap-4.10.0/pkg/wedderga-4.9.5-Push56/lib/main.gi:1396 called from SimpleAlgebraInfoByData( i ) at /proc/cygdrive/C/gap-4.10.0/pkg/wedderga-4.9.5-Push56/lib/main.gi:194 called from <function "unknown">( ) called from read-eval loop at stdin:3 you can 'return;' after assigning a value

[angeldelriomateos]

I discovered that the issue occurs with the combination of IsCyclotomicExtension and ReducingCyclotomicAlgebra. This is independent of the Shortening of WedderburnDecompositionInfo, i.e. it affects also to GlobalSplittingOfCyclotomicAlgebra. I think that I know how to fix it but I need time to do it properly. So my next pull request will be fixing this bug

Ok! Let us keep the terminology GlobalSplitting, it makes sense.

[angeldelriomateos]

I discovered that the issue occurs with the combination of IsCyclotomicExtension and ReducingCyclotomicAlgebra. This is independent of the Shortening of WedderburnDecompositionInfo, i.e. it affects also to GlobalSplittingOfCyclotomicAlgebra. I think that I know how to fix it but I need time to do it properly. So my next pull request will be fixing this bug

When I tried to load wedderga after pulling #62 the LoadPackage failed the first time. Using GlobalSplitting in main.gi seems an issue because the command hasn't been defined yet. After I loaded a second time, it was. On SG(480,266) it seemed to give a sequencing error:

Error, Variable: 'GlobalSplittingOfCyclotomicAlgebra' must have an assigned value in return GlobalSplittingOfCyclotomicAlgebra( A ) ; at /proc/cygdrive/C/gap-4.10.0/pkg/wedderga-4.9.5-Push56/lib/main.gi:1396 called from SimpleAlgebraInfoByData( i ) at /proc/cygdrive/C/gap-4.10.0/pkg/wedderga-4.9.5-Push56/lib/main.gi:194 called from <function "unknown">( ) called from read-eval loop at stdin:3 you can 'return;' after assigning a value

I think that loading wedderga shouldn't be a problem if your div-alg.gd is loaded with the declaration of GlobalSplittingOfCyclotomicAlgebra. I found out that the issue is the definition of the fields F1 and F2 in the code of ReducingCyclotomicAlgebra. I am trying to figure out how to fix this.

[drallenherman]

I discovered that the issue occurs with the combination of IsCyclotomicExtension and ReducingCyclotomicAlgebra. This is independent of the Shortening of WedderburnDecompositionInfo, i.e. it affects also to GlobalSplittingOfCyclotomicAlgebra. I think that I know how to fix it but I need time to do it properly. So my next pull request will be fixing this bug

When I tried to load wedderga after pulling #62 the LoadPackage failed the first time. Using GlobalSplitting in main.gi seems an issue because the command hasn't been defined yet. After I loaded a second time, it was. On SG(480,266) it seemed to give a sequencing error: Error, Variable: 'GlobalSplittingOfCyclotomicAlgebra' must have an assigned value in return GlobalSplittingOfCyclotomicAlgebra( A ) ; at /proc/cygdrive/C/gap-4.10.0/pkg/wedderga-4.9.5-Push56/lib/main.gi:1396 called from SimpleAlgebraInfoByData( i ) at /proc/cygdrive/C/gap-4.10.0/pkg/wedderga-4.9.5-Push56/lib/main.gi:194 called from <function "unknown">( ) called from read-eval loop at stdin:3 you can 'return;' after assigning a value

I think that loading wedderga shouldn't be a problem if your div-alg.gd is loaded with the declaration of GlobalSplittingOfCyclotomicAlgebra. I found out that the issue is the definition of the fields F1 and F2 in the code of ReducingCyclotomicAlgebra. I am trying to figure out how to fix this.

There is a problem with div-alg.gd - ALL of our new functions are missing! Is the problem that past changes are not being kept again?

[drallenherman]

If you revise the changes in main.gi, more precisely at the end of the code for SimpleAlgebraInfoByData you will find some commented lines. This is prepared for the second alternative. I encountered an issue with that alternative. I wait for your reaction on this full request before going ahead with the second alternative.

There could be an issue that would arise if we try to implement FullWDI one component at a time. In some cases SchurIndex(A) will have to call on SimpleComponentOfGroupRingByCharacter which then calls on SimpleAlgebraInfoByData - if this then calls on SchurIndex(A) again it would be trapped in an infinite loop in some cases. I think this command can first do WDI with GlobalSplitting, then then finish by computing the SchurIndex of each component and split the components with Schur index 1.

[drallenherman]

Upps. I discovered that WedderburnDecompositionInfo fails for SG(480,266). I am going to investigate what is the problem. At this moment I guess the problem is in GlobalSplittingOfCyclotomicAlgebra

I went back to the PR#59 branch to check this there - it's not a problem there. Did you check that your div.alg.gd in PR#62 isn't missing the new functions?

I want to adjust SchurIndexByCharacter to make use of SimpleComponentByCharacterDescent. On my computer with version #59 it calculated the Schur index of the character of largest degree of GL(3,3) in 79 seconds. Using SimpleComponentOfGroupRingByCharacter the same calculation took 606 seconds.

[angeldelriomateos]

Upps. I discovered that WedderburnDecompositionInfo fails for SG(480,266). I am going to investigate what is the problem. At this moment I guess the problem is in GlobalSplittingOfCyclotomicAlgebra

I went back to the PR#59 branch to check this there - it's not a problem there. Did you check that your div.alg.gd in PR#62 isn't missing the new functions?

I want to adjust SchurIndexByCharacter to make use of SimpleComponentByCharacterDescent. On my computer with version #59 it calculated the Schur index of the character of largest degree of GL(3,3) in 79 seconds. Using SimpleComponentOfGroupRingByCharacter the same calculation took 606 seconds.

I don't know how to see files inside pull request. I commit and pushed the changes in div.alg.gd long time ago. I have checked div.alg.gd in divalg branch and it contains the declarations of the new functions IsCyclotomicAlgebra and ReducingCyclotomicAlgebra.

[olexandr-konovalov]

@angeldelriomateos @drallenherman all problems with loading the code from this PR are due to the mix of master and divalg. This PR requires some change - and the advice which change to made is depending on whether you want the modification of main.gi to go to the master branch (so it may be released independently of divalg) or to the divalg branch.

[angeldelriomateos]

Thanks! Just to check, is this PR intended for the master branch or for divalg? It has two commits, one modifies div-alg.gi and it seems to me that it should not be here. It actually contains a huge change, which does not match the commit message (Replace F2 by F1 in ReducingCyclotomicExtension). The other commit modifies main.gi and it seems to me that this is what you are actually trying to achieve - is this the case?

The commit F2->F1 was done long time ago in divalg and I though that it was already estabilized and consists in a minimal change in function ReducingCyclotomicAlgebra inside div-alg.gi. The last commit is inside main.gi. After I did it I noticed that there was a bug in ReducingCyclotomicAlgebra. I am trying to fix this bug now

[angeldelriomateos]

Independently, Allen is having problems with the declaration of IsCyclotomicExtension and ReducingCyclotomicAlgebra in div-alg.gd. I don't know why.

[olexandr-konovalov]

@angeldelriomateos but the commit F2->F1 was a tiny fix in divalg - while now if you look at commit 3b9ef16c27421307cf2e6ae944230a340e55878d is contains a huge change in div-alg.gi. That does not seem right to me.

[olexandr-konovalov]

@drallenherman's the problems you see are exactly because of the mix-up of two branches, don't try to fix them, this PR needs to be updated first (In general, I usually not rush to check out a code from a PR until I will see the results of Travis CI tests). How to fix this PR depends on your intentions in main.gi - my understanding is that you want to change it independently of div-alg.gi changes, is that the case? If so, we will merge this into master, and then I will rebase the divalg branch so that the main.gi change will be visible in divalg.

[olexandr-konovalov]

P.S. @angeldelriomateos wrote

I don't know how to see files inside pull request.

The header of the pull request page has a tab "Files changed":

You can see it in the bottom right corner of this screenshot.

[drallenherman]

@drallenherman's the problems you see are exactly because of the mix-up of two branches, don't try to fix them, this PR needs to be updated first (In general, I usually not rush to check out a code from a PR until I will see the results of Travis CI tests). How to fix this PR depends on your intentions in main.gi - my understanding is that you want to change it independently of div-alg.gi changes, is that the case? If so, we will merge this into master, and then I will rebase the divalg branch so that the main.gi change will be visible in divalg.

I re-set my div-alg.gd manually from #59 just to get wedderga to Load so that I could test other things, I won't push any changes until I fetch the next pull request after you rebase it. Now I am seeing the error Angel had with WDI on SG(480,266).

[drallenherman]

Independently, Allen is having problems with the declaration of IsCyclotomicExtension and ReducingCyclotomicAlgebra in div-alg.gd. I don't know why.

It was more than that, none of the functions we added since March were there. This is due to a branch mix up according to Alex so it will get fixed.

[olexandr-konovalov]

I re-set my div-alg.gd manually from #59 just to get wedderga to Load so that I could test other things,

This may work in this case (#59 and divalg branch have same div-alg.gd), but may be counterproductive in general, especially if you will do git commit -a to commit all modified files. A better model is to think in terms of branches.

I won't push any changes until I fetch the next pull request after you rebase it. Now I am seeing the error Angel had with WDI on SG(480,266).

For example, using branches does not prevent you from making changes in a branch, and pushing it.

It was more than that, none of the functions we added since March were there.

I think because of the branch mix-up

This is due to a branch mix up according to Alex so it will get fixed.

As soon as you will let me know to which branch this PR is actually intended!

[drallenherman]

I re-set my div-alg.gd manually from #59 just to get wedderga to Load so that I could test other things,

This may work in this case (#59 and divalg branch have same div-alg.gd), but may be counterproductive in general, especially if you will do git commit -a to commit all modified files. A better model is to think in terms of branches.

I won't push any changes until I fetch the next pull request after you rebase it. Now I am seeing the error Angel had with WDI on SG(480,266).

For example, using branches does not prevent you from making changes in a branch, and pushing it.

It was more than that, none of the functions we added since March were there.

I think because of the branch mix-up

This is due to a branch mix up according to Alex so it will get fixed.

As soon as you will let me know to which branch this PR is actually intended!

62 had this issue with div-alg.gd when I fetched it this morning. I thought every other branch was closed.

[olexandr-konovalov]

@drallenherman I see. Look at https://github.com/gap-packages/wedderga/pulls and you will see what's currently open. Also, we will not close divalg for a while, since we are submitting pull requests to it until it will stabilise - so PR #58 will remain open for now.

[angeldelriomateos]

Hi, I think that I have fixed the issue in ReducingCyclotomicAlgebra. I have checked with SG(480,166) and it seems to work. This should be part of a pull request in divalg because this is a function of div-alg.gi

[angeldelriomateos]

However I am afraid to interfere in the fixing you two are doing there. So, please make sure that we all have the same version, that in my opinion is what I had this morning.

[olexandr-konovalov]

@angeldelriomateos let's do one step at a time. Looking at https://github.com/angeldelriomateos/wedderga/commits/ShortWDI, I see that this PR is based on the master branch:

However, it uses GlobalSplittingOfCyclotomicAlgebra which is only present in the divalg branch. So, it should be submitted to the divalg branch, isn't it?

[angeldelriomateos]

The changes in that pull request affect to main.gi where one make use to GlobalSplittingOfCyclotomicAlgebra.

[angeldelriomateos]

I created a new branch ShortWDI hanging from master because I considered this as independent of divalg. Probably this is wrong, because it uses functions which are in divalg, and have not been transferred to master yet. What should I do now?

[angeldelriomateos]

At this moment I would like to start a pull request to fix the bug in ReducingCyclotomicExtension. This is independent of ShortWDI because it also affect to what we were doing before I started ShortWDI. However it affects also to ShortWDI

[olexandr-konovalov]

I created a new branch ShortWDI hanging from master because I considered this as independent of divalg. Probably this is wrong, because it uses functions which are in divalg, and have not been transferred to master yet. What should I do now?

Perfect, now it makes complete sense to me! This is how to fix this with minimal efforts: create a new branch off divalg, cherry-pick the commit we need, and then submit a new pull request:

git checkout divalg
git pull origin divalg
git checkout -b ShortWDI-divalg
git cherry-pick 8563e3ffbd6e257c9b14cbba3105822f113947c2
git push angeldelriomateos

and now go to https://github.com/gap-packages/wedderga and submit new pull request to divalg.

[olexandr-konovalov]

At this moment I would like to start a pull request to fix the bug in ReducingCyclotomicExtension. This is independent of ShortWDI because it also affect to what we were doing before I started ShortWDI. However it affects also to ShortWDI

haven't noticed this earlier - in this case, I suggest to do things above to submit a new PR for ShortWDI to divalg, and then go back to divalg or to master (depending on where you'd like to make a PR) and work on a new PR. Later, when ReducingCyclotomicExtension bug will be fixed, I will help to rebase the ShortWDI-divalg branch to make that bugfix visible there.

[angeldelriomateos]

I created a new branch ShortWDI hanging from master because I considered this as independent of divalg. Probably this is wrong, because it uses functions which are in divalg, and have not been transferred to master yet. What should I do now?

Perfect, now it makes complete sense to me! This is how to fix this with minimal efforts: create a new branch off divalg, cherry-pick the commit we need, and then submit a new pull request:

git checkout divalg
git pull origin divalg
git checkout -b ShortWDI-divalg
git cherry-pick 8563e3ffbd6e257c9b14cbba3105822f113947c2
git push angeldelriomateos

and now go to https://github.com/gap-packages/wedderga and submit new pull request to divalg.

Done

[olexandr-konovalov]

Thanks, it worked! So, I am closing this, as it is now replalced by #63.