Skip to content

Implement MultVectorRight for lists#6280

Merged
ThomasBreuer merged 1 commit intomasterfrom
mh/MultVectorRight
Mar 25, 2026
Merged

Implement MultVectorRight for lists#6280
ThomasBreuer merged 1 commit intomasterfrom
mh/MultVectorRight

Conversation

@fingolfin
Copy link
Member

@fingolfin fingolfin commented Mar 24, 2026

AI assistance: Codex prepared the test updates.

Co-authored-by: Pavol Kollár palkollar9@gmail.com
Co-authored-by: Max Horn max@quendi.de
Co-authored-by: Codex codex@openai.com

Extracted from PR #5980

@fingolfin fingolfin requested a review from ThomasBreuer March 24, 2026 12:20
@fingolfin fingolfin changed the title Implemented the method MultVectorRight for lists Implement MultVectorRight for lists Mar 24, 2026
@fingolfin fingolfin added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: library release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels Mar 24, 2026
Copy link
Contributor

@ThomasBreuer ThomasBreuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor problem with GAPDoc syntax and with the formulation of a sentence.

(It is funny that the kernel function MULT_VECTOR_RIGHT_2 was already available, but was used only for IsPlistVectorRep.)

lib/listcoef.gd Outdated
## This operation calculates <A>mul</A>*<A>list1</A> in-place.
## These operations multiply the entries of <A>list1</A> by <A>mul</A> in-place.
## The operation <Ref Oper="MultVectorLeft"/> calculates <A>mul \cdot list1</A> and
## the operation <Ref Oper="MultVectorRight"/> calculates <A>list1 \cdot mul</A>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not allowed GAPDoc syntax for <A> elements.
And the description is not correct: If both mul and list1 are lists of nesting depth 1 then mul * list1 and list1 * mul sum up the pointwise products, due to the rules of list arithmetics, but the MultVector functions compute the list of products of the elements of list1 with mul.
(And we could rename list1 to list.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch, indeed, I totally missed that the AI produced nonsense here (not an excuse, the blame is on me).

I've pushed an improved version.

AI assistance: Codex prepared the test updates.

Co-authored-by: Pavol Kollár <palkollar9@gmail.com>
Co-authored-by: Max Horn <max@quendi.de>
Co-authored-by: Codex <codex@openai.com>
@fingolfin fingolfin force-pushed the mh/MultVectorRight branch from b362f43 to 49a2283 Compare March 24, 2026 23:58
@fingolfin fingolfin requested a review from ThomasBreuer March 25, 2026 07:42
Copy link
Contributor

@ThomasBreuer ThomasBreuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good.

@ThomasBreuer ThomasBreuer merged commit 68e124d into master Mar 25, 2026
32 checks passed
@ThomasBreuer ThomasBreuer deleted the mh/MultVectorRight branch March 25, 2026 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: library

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants