oscar-system / Oscar.jl

A comprehensive open source computer algebra system for computations in algebra, geometry, and number theory.
https://www.oscar-system.org
Other
305 stars 112 forks source link

`map_word` ignores `init` in some cases #3885

Open fieker opened 4 days ago

fieker commented 4 days ago
G = abelian_group(FPGroup, [2])
A = abelian_group(2)

then

map_word(one(G), gens(A), init = zero(A))

will fail as it calls for one(A[1]) which does not exist. It ignores the init....

lgoettgens commented 4 days ago

I think @ThomasBreuer last worked on this function, maybe you could have a look?

ThomasBreuer commented 4 days ago

Yes, we should proceed as sketched at the end of discussion in #3796. I will provide a pull request.

ThomasBreuer commented 4 days ago

@fieker Currently map_word does not work at all for evaluating a word in a vector of FinGenAbGroupElems. We need a separate method for them that translates multiplication and powering into addition and scalar multiplication. (In this sense, this particular problem is not one of a missing init handling.) Other generic algorithms (which ones?) will fail for FinGenAbGroupElems for the same reason.

thofma commented 4 days ago

I think we encountered this before. Two options that I remember:

  1. We wrap/unwrap the abelian group (doing the opposite of MultGrp/MultGrpElem). For example
    function map_word(g, v::Vector{FinGenAbGroupElem})
    unwrap(g, wrap.(v))
  2. We do some _op, _inv shenanigans where we rewrite the relevant methods and replace x * y with _op(x, y) etc. and define
    _op(x::GAPGroupElem, y::GAPGroupElem) = x * y
    _op(x::FinGenAbGroupElem, y::FinGenAbGroupElem) = x + y

    etc.

Edit: There is of course the obvious solution. Just define another map_word(g, v::Vector{FinGenAbGroupElem}) method and use + instead of * etc.

fieker commented 4 days ago

On Mon, Jun 24, 2024 at 09:04:05AM -0700, Thomas Breuer wrote:

@fieker Currently map_word does not work at all for evaluating a word in a vector of FinGenAbGroupElems. We need a separate method for them that translates multiplication and powering into addition and scalar multiplication. (In this sense, this particular problem is not one of a missing init handling.) Other generic algorithms (which ones?) will fail for FinGenAbGroupElems for the same reason.

I agree - but I'd still like to not ignore init... - or be honest and not provide the option at all -- Reply to this email directly or view it on GitHub: https://github.com/oscar-system/Oscar.jl/issues/3885#issuecomment-2186916549 You are receiving this because you were mentioned.

Message ID: @.***>

fieker commented 4 days ago

On Mon, Jun 24, 2024 at 09:16:43AM -0700, Tommy Hofmann wrote:

I think we encountered this before. Two options that I remember:

  1. We wrap/unwrap the abelian group (doing the opposite of MultGrp/MultGrpElem). For example
    function map_word(g, v::Vector{FinGenAbGroupElem})
    unwrap(g, wrap.(v))
  2. We do some _op, _inv shenanigans where we rewrite the relevant methods and replace x * y with _op(x, y) etc. and define
    _op(x::GAPGroupElem, y::GAPGroupElem) = x * y
    _op(x::FinGenAbGroupElem, y::FinGenAbGroupElem) = x + y

    etc.

The abelian group stuff is irrelevant for this, was a bad example by me. My complaint really is about ignoring the init argument and instead of blindly relying that one(s.th.) does the correct thing. The other question that was asked is if init needs to be used if present as this might also change type and results, or if init may be ignored if not needed.

My original problem was map_word(GrpElem, Vector{Map}) where, in general, we do not have one(Map). In GModule(s) I've added this for abelian groups and AA.Generic.Modules, but e.g. ERROR: MethodError: no method matching one(::NumFieldHom{…}) Hence this discussion...

The "reverse" to MultGroup is a different thing and might be useful.

-- Reply to this email directly or view it on GitHub: https://github.com/oscar-system/Oscar.jl/issues/3885#issuecomment-2186941891 You are receiving this because you were mentioned.

Message ID: @.***>

fingolfin commented 4 days ago

Julia always honors init if given:

julia> sum(1:10; init=5)
60

julia> prod(1:10; init=0)
0

so I think we should mimic that.

ThomasBreuer commented 4 days ago

@fingolfin The documentation of prod emphasizes that

it is unspecified whether `init` is used for non-empty collections.

I think it is a good idea to initialize the product in map_word with a given init value. Then we document this for map_word. (We cannot implement this behaviour by just calling prod on the image syllables and with the given init value.)

@thofma Yes, I am going to provide map_word(g, v::Vector{FinGenAbGroupElem}) with "additive meaning".