missing input sanity check for `hom`

[YueRen]

Found with @oskarhenriksson. It is currently possible to construct hom whose images do not lie in the specified codomain:

julia> Qt,t = rational_function_field(QQ,"t")
(Rational function field over QQ, t)

julia> Qtx,(x1,x2) = polynomial_ring(Qt,["x1","x2"])
(Multivariate polynomial ring in 2 variables over rational function field, AbstractAlgebra.Generic.MPoly{AbstractAlgebra.Generic.RationalFunctionFieldElem{QQFieldElem, QQPolyRingElem}}[x1, x2])

julia> f = one(Qtx)
1

julia> R,(t,x1,x2) = polynomial_ring(QQ,["t","x1","x2"])
(Multivariate polynomial ring in 3 variables over QQ, QQMPolyRingElem[t, x1, x2])

julia> phi = hom(Qtx, R, c->evaluate(c,t), [x1,x2])
Ring homomorphism
  from multivariate polynomial ring in 2 variables over rational function field
  to multivariate polynomial ring in 3 variables over QQ
defined by
  x1 -> x1
  x2 -> x2
with map on coefficients
  #11

julia> parent(phi(f)) # not in R!!!
Fraction field
  of multivariate polynomial ring in 3 variables over QQ

The main error is of course that the hom I am trying to construct is nonsense (in t in Qtx may have a negative exponent, t in R cannot). But it would be nice if OSCAR could tell me that during the constructing of the hom (e.g., by testing whether phi(one(domain)) lies in the codomain) or at the latest when I try to use it.

[thofma]

I guess we should assert that phi(f)is indeed in the codomain. In case the coefficient ring map is specified by an arbitrary (julia) function, there is not much else we can do. It is easy to construct an example, where phi(one(domain)) is in the codomain, but not phi(2*one(domain)).

Can you adjust the title to be a bit more descriptive? There is no error in hom. I would guess it is a typical case of "garbage in, garbage out" and Oscar should try a bit harder at catching the "gargabe out" part.

[mjrodgers]

I think this is a mostly unavoidable problem, for example Magma also doesn't check when you are creating a hom whether the map provided actually is a hom. I think it would create too much overhead to do any meaningful check (for example, as mentioned, checking that phi(one(domain)) is in the codomain doesn't guarantee much of anything overall).

It seems like it could be really costly to check that phi(x) is in the codomain every time the map was invoked.

[YueRen]

How about introducing an optional check that is enabled by default then? And just to be clear, we are only talking about those hom where we cannot guarantee that the image lands in the codomain (most likely due to a bad coefficient map).

[thofma]

It seems like it could be really costly to check that phi(x) is in the codomain every time the map was invoked.

I am not sure about other types, but for ring homomorphisms like here, it is super cheap (a comparison of pointers) to check if phi(x) is in the codomain, since phi(x) will know its parent and it is either the codomain or not.

Edit: We could make the error more helpful by printing the parent of phi(x) and what was expected (codomain(phi)).

[fingolfin]

So what is the suggestion: that phi(x) after computing the image then adds a check (possibly an @assert) to verify that the value it produces has the right parent?

If this was to be made optional, then the homomorphism would have to store whether it should to the check or not, and then use that conditionally... (An alternative implementation strategy could also use different types). But since it is so cheap to test parents, perhaps one could just always do it?

But of course there are many homomorphism implementations in Oscar, adjusting all of them will be quite some work (just to find all of them may be tricky...)

[thofma]

Just add the check parent(phi(x)) == unconditionally.