Possible issues in ModuleHomomorphism and ModuleIsomorphism constructors

[diracdeltafunk]

It is possible to construct a ModuleHomomorphism between two FPModules which is not mathematically valid. For example, one can construct a map $\mathbb{Z}/2 \to \mathbb{Z}/4$ which sends $1$ to $1$:

using AbstractAlgebra

function cyclic_module(n)
   F = free_module(ZZ, 1)
   S, _ = sub(F, [n*gen(F, 1)])
   return quo(F, S)[1]
end

Z2 = cyclic_module(2)
Z4 = cyclic_module(4)

f = ModuleHomomorphism(Z2, Z4, matrix(ZZ, 1, 1, [1]))

image_of_relation = Z4(relations(Z2)[1] * matrix(f))
@show image_of_relation
@show iszero(image_of_relation)

This constructs successfully, but the defining relation of $\mathbb{Z}/2$ maps to a nonzero element of $\mathbb{Z}/4$:

image_of_relation = (2)
iszero(image_of_relation) = false

So the matrix does not define a well-defined homomorphism on the quotient module.

A related issue occurs for ModuleIsomorphism. For example, the quotient map $\mathbb{Z}/4 \to \mathbb{Z}/2$ sending $1$ to $1$ is a valid homomorphism, but it is not an isomorphism:

psi = ModuleIsomorphism(Z4, Z2, matrix(ZZ, 1, 1, [1]))

x = 2*gen(Z4, 1)
@show x
@show iszero(x)
@show psi(x)
@show inv(psi)(psi(x))
@show inv(psi)(psi(x)) == x

This also constructs successfully, but the computed inverse is not actually inverse on elements:

x = (2)
iszero(x) = false
psi(x) = (0)
inv(psi)(psi(x)) = (0)
inv(psi)(psi(x)) == x = false

Is this behavior expected? If so, it would be nice to make a note of this in the documentation -- this was certainly counter to my expectations when I started using AbstractAlgebra.jl. If not, I would be happy to submit a PR to change the behavior.

I would have expected constructors for homomorphisms between presented modules to verify that source relations are sent to zero in the target. Likewise, I would expect the constructor for isomorphisms to additionally verify that the computed inverse is a well-defined homomorphism and that the two composites are identities modulo the relevant relations, or otherwise reject the input.

Some implementation notes from a quick look:

• The generic ModuleHomomorphism constructor checks matrix dimensions, but does not check that source relations map to zero.
• The public ModuleHomomorphism(M1, M2, m::MatElem) method appears to call the inner Generic.ModuleHomomorphism{T} constructor directly, bypassing even the generic dimension check.
• hom(...; check=true) accepts a check keyword, but the keyword does not appear to be used for this validation.
• The ModuleIsomorphism constructor seems to solve for a one-sided inverse using the target relations, but does not rule out non-injective maps such as $\mathbb{Z}/4 \to \mathbb{Z}/2$.

Disclaimer The text of this issue was written partially by Codex (and partially by me).