Fix bug in MakeImagesInfoLinearGeneralMappingByImages that could lead to errors resp. wrong results if multiplication with coefficients from the right is not defined resp. not commutative

[cdwensley]

Description

This one-line change to lib/vspchom.gi attempts to fix issue #4481.
The fix is just to change the order of vectors in LinearCombination( x, maxi[2] );

Text for release notes

Fixed the construction of linear mappings by images in certain situations where it is essential that multiplying elements in the image with coefficients happens from the left, not from the right. (Note that multiplication from the left is assumed, but in many computations it does not make a difference.)
Before the fix, one got an error if multiplying with coefficients from the right is not defined, and one got a perhaps wrong result if this multiplication is defined but not commutative.
Examples of the error case can be constructed with algebras generated by algebra homomorphisms as images of the linear mapping.

(End of text for release notes)

Checklist for pull request reviewers

• proper formatting

If your code contains kernel C code, run clang-format on it; the
simplest way is to use git clang-format, e.g. like this (don't
forget to commit the resulting changes):

git clang-format $(git merge-base HEAD master)

• usage of relevant labels

  1. either release notes: not needed or release notes: to be added
  2. at least one of the labels bug or enhancement or new feature
  3. for changes meant to be backported to stable-4.X add the backport-to-4.X label
  4. consider adding any of the labels build system, documentation, kernel, library, tests

• runnable tests

• lines changed in commits are sufficiently covered by the tests

• adequate pull request title

• well formulated text for release notes

• relevant documentation updates

• sensible comments in the code

[cdwensley]

This fix for issue #383, or some alternative, is required by the XModAlg package

[fingolfin]

Issue #383 is about "Fix version header dependency"... ?!

[cdwensley]

Apologies, it is issue #4481 (number 383 of the open issues).

[wilfwilson]

Thanks for spotting this.

If you can see a way of reasonably doing this, could you please add a test (to either tst/testinstall/vspchom.tst, or to a new file in tst/testbugfix/ following the style there) that demonstrates the now-correct behaviour?

Ideally it would be a test that would fail/give an error/give the wrong answer in the current master branch, but which of course gives the right answer with this PR.

[wilfwilson]

Thank you, nice.

[cdwensley]

The test 2021-05-11-LeftModHom.tst has been added to tst/testbugfix, and all checks are still passing.
The actual fix was first tried out on a "let's see if this works" basis, without any understanding why, so it would be best if someone with a good knowledge of algebras in GAP had a look, e.g. @ThomasBreuer.

[ThomasBreuer]

Thanks, @cdwensley, for the fix.

Yes, the ordering of arguments in the code line in question was wrong.
I have looked through the instances of LinearCombination( in the GAP library, all except the one which you found look correct.

I think that the bug can also lead to wrong results (not only to error messages) if one considers suitable row vector spaces over noncommutative rings.

[fingolfin]

I think we need a different text for the release notes; MakeImagesInfoLinearGeneralMappingByImages is an undocumented internal function, telling users that it has been fixed is IMHO problematic, as it might cause people to think that it is documented now, while at the same time not really telling them how to determine whether it might have affected them.

So instead we should list documented functions that were directly (or indirectly?) affected by this

[cdwensley]

How about "Fix for AlgebraGeneralMappingByImages when one of the algebras is generated by algebra homomorphisms."? (I've no idea how to change the "Text for release notes".)

[ThomasBreuer]

I have edited the text for the release notes, please check if it is sufficient now.

[cdwensley]

That looks very thorough.