gap-packages / wedderga

Wedderburn Decomposition of Group Algebras
https://gap-packages.github.io/wedderga/
GNU General Public License v2.0
3 stars 6 forks source link

Adjusted SchurIndexByCharacter to use character descent #66

Closed drallenherman closed 4 years ago

drallenherman commented 4 years ago

Adjusted SchurIndexByCharacter to use character descent; fixed bug in DefiningCharacterOfCyclotomicAlgebra; revised documentation - Issue #65

UPDATE: replaced by PR #67

olexandr-konovalov commented 4 years ago

Thanks @drallenherman - you're submitting this to master branch, hence it has 27 commits in it! This should be submitted to divalg branch - you can change the base branch of this PR from master to divalg as explained https://github.blog/2016-08-15-change-the-base-branch-of-a-pull-request/ or I can update it for you.

You can also see below "This branch cannot be rebased due to conflicts" - if this is visible straight after PR submission, then something is wrong...

I have also renamed this PR, to have a title less cryptic than Pr/59:)

drallenherman commented 4 years ago

Thanks @drallenherman - you're submitting this to master branch, hence it has 27 commits in it! This should be submitted to divalg branch - you can change the base branch of this PR from master to divalg as explained https://github.blog/2016-08-15-change-the-base-branch-of-a-pull-request/ or I can update it for you.

You can also see below "This branch cannot be rebased due to conflicts" - if this is visible straight after PR submission, then something is wrong...

I have also renamed this PR, to have a title less cryptic than Pr/59:)

I suppose this is because PR#59 was a branch off master not divalg. Glad you could change the title. I've finished my modifications for this issue now.

olexandr-konovalov commented 4 years ago

No, I started #59 off divalg. If you are done with this for now, I will try change the base branch of this PR to make it looking like a PR to divalg.

drallenherman commented 4 years ago

No, I started #59 off divalg. If you are done with this for now, I will try change the base branch of this PR to make it looking like a PR to divalg.

Yes done for now. Thanks. Go ahead.

drallenherman commented 4 years ago

No, I started #59 off divalg. If you are done with this for now, I will try change the base branch of this PR to make it looking like a PR to divalg.

Yes done for now. Thanks. Go ahead.

travis tests were crashing due to a missing line in DefiningCharacterOfCyclotomicAlgebra, two commits now to fix. Not sure why one is not finishing.

olexandr-konovalov commented 4 years ago

You can click on Travis build to see the log and figure out where it's stuck. We observe that from time to time it takes a lot of time to download GAP packages archive. Sometimes restarting a build in Travis helps.

olexandr-konovalov commented 4 years ago

I've changed this PR to be merged into divalg, number of commits reduced from 27 to 18, but it's still too convoluted. It need to be rebased on the latest divalg - my understanding it that from all list of commits at https://github.com/gap-packages/wedderga/pull/66/commits only todays work matters, right?

drallenherman commented 4 years ago

I've changed this PR to be merged into divalg, number of commits reduced from 27 to 18, but it's still too convoluted. It need to be rebased on the latest divalg - my understanding it that from all list of commits at https://github.com/gap-packages/wedderga/pull/66/commits only todays work matters, right?

Yes. Only today.

olexandr-konovalov commented 4 years ago

Closed in favour of the 2nd attempt in PR #67.