compose(f,g) is inconsistent

[fingolfin]

We need to settle what that means, ensure it is implemented consistently everywhere, and documented clearly.

There are two choices how to interpret it, both are natural in some settings but not in others:

• compose(f,g)(x) = f(g(x)) is compatible with left application of functions and matches Julia's \circ operator, which satisfies (f ∘ g)(x) = f(g(x)); this is what textbooks use nowadays
• compose(f,g)(x) = g(f(x)) is compatible with right application of functions (so f(x) = x^f and then x^{compose(f,g)} = (x^f)^g); for this composition I have often seen the convention to denote it by f g (juxtaposition), sometimes alsof \cdot g or f * g) -- this is by the way historically the older notation, and e.g. group theorists still use it a lot

But in Oscar, what compose does differs by type, which is bad. We need to settle on one and implement it uniformly. I'll try to sum up the state below; I only looked at the written documentation for this, though; what is actually implemented might differ yet again...

Places that say compose(f,g) = g(f(x) = (x^f)^g:

• the AbstractAlgebra manual section for the Map interface
• it also defines f*g as a shorthand for that (remember that f\circ g already exists as shorthand for "the other order" in Julia, at least for functions; IMHO it would make sense to also implement it for OSCAR stuff, for consistency, but that's a separate issue)
• this is consistent with other places in the AA manual
• compose for AlgHom{T}
• based on code, compose for AbsSpaceMor in Hecke.jl

Places that say compose(f,g) = f(g(x) = (x^g)^f:

• compose for PolyElem in AA
• compose for RelSeriesElem in AA
• compose for (GAP) groups in Oscar (now fixed via PR changed compose in src/Groups ... #1038)

Since the AA documentation is quite explicit on this, I tend to say we should follow it; what worries me is that some docstrings in AA claim to do it the other way around; but perhaps the doc strings are wrong?

Besides settling this, I think it'd be a great thing to have a conformance test for, though I am not sure how to best to arrange for it.

CC @wbhart @thofma @fieker

[fingolfin]

Discussed with @fieker and I think we agree that compose should do what it claims in the AA docs; so now we need to fix everything that does it wrong.

[fingolfin]

This is now fixed for (GAP) groups, by PR #1038.

[lgoettgens]

citing @fieker from slack:

I'm not sure how much of an issue it is. For all Map-types compose(a,b) is b(a(x))
for non-map types, I'd never use compose... I'd be in favour of droppnig compse for non-maps amd use evaluare instead. or even f(a). THis is non-ambigous

[lgoettgens]

Nemocas/Nemo.jl#1521 implements the changes proposed here for all remaining Nemo types. But it still has the same seriousness of breaking a lot of stuff as Nemocas/AbstractAlgebra.jl#1025 for PolyElem and RelSeriesElem.

How to proceed here?

1. Breaking change in AA and Nemo
2. Remove compose for all non-map types (as suggested by @fieker)
3. ?

[thofma]

Since it is called polynomial composition, I would prefer to keep compose for polynomials for the sake of discoverability.

One option to go forward would be to

1. remove all of our uses of compose for non-map types and make new releases.
2. introduce a warning in the compose for non-map types, that the order will change in the next version.
3. make a breaking release, where we change the order of compose for non-map types.

This is the only thing I could come up with.

[thofma]

After the discussions we had a while ago, the following is the best I can come up with:

1. Turn compose(f::PolyElem, g::PolyElem) into compose(f::PolyElem, g::PolyElem; side) with a mandatory keyword argument. Similar for the other problematic method. Maybe we could also introduce a _compose with the right behaviour that we can use internally (or whatever name we settle on. I don't care too much).
2. Wait a few years.
3. Add the default keyword.
4. Profit.

We have dallied over this quite a bit, so unfortunately 2. would not finish before 1.0. On ther other hand, we can also just promote f(g) resp. g(f) over compose both in our code as well as in the documentation.

Any comments @fingolfin @fieker?

[fieker]

On Sat, Dec 30, 2023 at 10:49:04AM -0800, Tommy Hofmann wrote: After the discussions we had a while ago, the following is the best I can come up with: 1. Turn `compose(f::PolyElem, g::PolyElem)` into `compose(f::PolyElem, g::PolyElem; side)` with a *mandatory* keyword argument. Similar for the other problematic method. Maybe we could also introduce a `_compose` with the right behaviour that we can use internally (or whatever name we settle on. I don't care too much). 2. Wait a few years. 3. Add the default keyword. 4. Profit. We have dallied over this quite a bit, so unfortunately 2. would not finish before 1.0. On ther other hand, we can also just promote `f(g)` resp. `g(f)` over `compose` both in our code as well as in the documentation.

I don't like compose, but I guess it is a fixture in the literature. I'm in favour of f(g) evaluate(f, g) but also: polynomials are not maps, hence bahaviour can be different from maps. I'd go with 1 and the f(g) overload

…

Any comments @fingolfin @fieker? -- Reply to this email directly or view it on GitHub: #690 (comment) You are receiving this because you were mentioned. Message ID: ***@***.***>