Add Embedding and Projection for direct sums of algebras

[cdwensley]

Methods for DirectSumOfAlgebras do not yet provide embeddings or projections. This PR aims to remedy that.
The first commit just adds SetDirectSumInfo to the various methods.

[codecov]

Codecov Report

❌ Patch coverage is 94.50549% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.67%. Comparing base (3f74e5d) to head (058206d).

Files with missing lines	Patch %	Lines
lib/algebra.gi	92.64%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6618      +/-   ##
==========================================
- Coverage   79.67%   79.67%   -0.01%     
==========================================
  Files         684      684              
  Lines      295464   295517      +53     
  Branches     8656     8656              
==========================================
+ Hits       235421   235462      +41     
- Misses      58217    58229      +12     
  Partials     1826     1826              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cdwensley]

A tricky detail is that the direct sum of matrix algebras, for example, may be constructed using either basis vectors or algebra generators, so this needs to be stored in the info.

[fingolfin]

This needs tests

[cdwensley]

@fingolfin - indeed, and tst/testextra appears to be the best place?

[cdwensley]

Work on Projections stalled while waiting for a reply to my message to GAP support: "Why is this map not single valued?"

[cdwensley]

It appears that DirectProductInfo is not documented in the reference manual, so this should be added.

[cdwensley]

... or, perhaps, there was a decision to not document these sorts of Info?

[cdwensley]

Problem with Projections fixed: should use Zero(A), not One(A), as the image for the other generators.

[cdwensley]

ringsc.gd contains the declatation: DeclareAttribute( "DirectSumInfo", IsGroup, "mutable" );
Surely IsGroup should be IsRing, so making that change here.

[cdwensley]

Just realised that, in a DirectSumInfo of a ring, info.first starts at 0, not 1. So will change to that convention here.

[lgoettgens]

One of you recent pushes screwed up the history. It now contains a handful of commits from master. Could you please try to rebase your PR once you are happy with your changes?

[cdwensley]

No great surprise, as I do not understand the process of rebasing. I have just committed; rebased; and made two small commits. How does the history look to you now? Chris

[cdwensley]

Note that commit b352a74 also includes an implementation of Embedding and Projection for direct sums of rings, and documents DirectSumInfo as section 56.9.6.

[fingolfin]

I had a look at the branch. It contains 34 commits from master that were
re-committed on your machine, in two batches (1 Oct and 2 Oct). Together with
the commit "trying to push after a rebase", I think this is what happened, twice:

1. git rebase ... worked fine.
2. git push was rejected. That is expected: a rebase rewrites your commits,
   so the branch on GitHub no longer matches yours. Unfortunately git then
   suggests git pull.
3. git pull did the damage: it replayed the master commits on top of the
   old version of your branch.

So the rule is: after a rebase, never git pull. Push with
git push --force-with-lease instead.

Here is how to clean up. I tried it on a copy of your branch and it goes
through without conflicts. I assume the remote for gap-system/gap is called
origin; git remote -v tells you, substitute the name if it differs.

1. git status must say "nothing to commit, working tree clean". If not,
   commit your changes first.
2. git switch embed
3. git branch embed-backup. This is a safety net: whatever happens below,
   your work is still in embed-backup.
4. git fetch origin
5. git status should now say "Your branch is up to date with
   'origin/embed'" or "ahead of". If it says "behind" or "have diverged",
   stop and paste the output here.
6. git rebase origin/master. This prints many lines "warning: skipped
   previously applied commit ..."; those are the copies of master commits
   being dropped, which is what we want. It should end with "Successfully
   rebased and updated refs/heads/embed." If it stops with a conflict instead,
   run git rebase --abort and paste the output here.
7. git log --oneline origin/master.. should list 13 commits, all yours
   (more if you had unpushed ones).
8. git push --force-with-lease

In future, to bring the branch up to date with master:

git fetch origin
git rebase origin/master
git push --force-with-lease

This does not use your local master at all, so it does not matter whether
that one is up to date.

[cdwensley]

Many thanks for your very detailed instructions - sorry to cause you so much hassle.
Report on what happened when following these steps:
There is an 'origin', but also an 'upstream' which is what I have been using.

1. 'git status' always reports a bunch of untracked files, but that's no problem
2. 'git log --oneline upstream/master' produced pages of commits
3. 'git push --force-with-lease' produced an error: 'fatal: The current branch embed has no upstream branch....'
   but 'git push --force-with-lease upstream embed' produced:
   Enumerating objects: 95, done.
   Counting objects: 100% (95/95), done.
   Delta compression using up to 8 threads
   Compressing objects: 100% (82/82), done.
   Writing objects: 100% (82/82), 14.05 KiB | 4.68 MiB/s, done.
   Total 82 (delta 68), reused 0 (delta 0), pack-reused 0
   remote: Resolving deltas: 100% (68/68), completed with 13 local objects.
   To https://github.com/gap-system/gap
   • 7c46974...63e2727 embed -> embed (forced update)
     which appears to be satisfactory.

[fingolfin]

Thanks for working on this, embeddings and projections for direct sums of algebras have been missing for a long time (the #T comments you removed say so). The basic case works: for a sum of two algebras built from bases, Embedding and Projection onto the second summand do what they should.

It needs more work before it can be merged, though.

If you want, restrict this PR to algebras and do rings in a follow-up; that would make it quite a bit smaller.

Changing the declaration of DirectSumInfo from IsGroup to IsRing is right.

AI disclosure: I used AI to help review this PR.

[cdwensley]

Not sure why the tests are failing here - just seems to be a variation in line breaks?

[fingolfin]

Thanks, this is much better: the history is clean, and nine of my thirteen comments are dealt with, so I resolved those. Four are still open (list form, kernel of the projection, flattening in the Lie method, test file); I added replies and a few new inline comments.

About the failing checks. There are two causes, both easy to fix.

1. gaplint complains about lib/algebra.gi:3020: Unused local variables: info2, info1. Removing them from the local list fixes it.

2. All other failing checks show the same six differences in tst/testinstall/alg-dsum.tst. You are right that they are only line breaks, but Test compares the output character by character:

   • Test always formats output for a line width of 80, however wide your terminal is. Output copied from an interactive session in a window of another width, or re-wrapped by hand, has its line breaks elsewhere. For example line 161 of the file is 114 characters long.
   • An empty line directly before a gap> line counts as output of the previous command. That is the difference reported for lines 170 and 191.

   You do not need to fix this by hand. Start GAP in the root directory of your checkout and let it rewrite the file:

   gap> Test( "tst/testinstall/alg-dsum.tst", rec( rewriteToFile := "tst/testinstall/alg-dsum.tst" ) );
   

   This shows the six differences once more and replaces the expected output in the file by what GAP really prints. Afterwards

   gap> Test( "tst/testinstall/alg-dsum.tst" );
   

   must return true. git diff should then show changes in a handful of lines only. Commit and push as usual.

It is worth running that second command each time before you push a change to a test file. And if you want an empty line to separate two parts of a test file, put a line starting with # after it (not gap> #).

AI disclosure: I used Claude Code (Opus 5.5) to re-review the branch. It built it, ran the examples quoted in the comments, and drafted the comments.

[cdwensley]

Have attempted to deal with all the requests above, but have probably missed a few. See what happens when the changes are uploaded.
It seems to me that the algebras of type GroupRing( ... ) should also be processed using generators, rather than basis vectors, so that would require some changes, and then different examples which did use basis vectors.

[cdwensley]

Just attempted to rebase on current master using instructions from 4th August. Seems to have worked.
Will have to check whether the documentation looks OK.
The comment above says that there is 1 change requested, but what is it?