sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.44k stars 481 forks source link

Creating a module homomorphism with a matrix may build the wrong codomain #31818

Closed toadrush closed 3 years ago

toadrush commented 3 years ago

Creating a free module morphism with a matrix and without specifying its codomain, currently creates a codomain spanned by row of the matrix.

This causes a problem when creating a vector-space homomorphism with an integer matrix.

sage: V = QQ^2; m = identity_matrix(2)                                          
sage: V.hom(m)                                                                  
Free module morphism defined by the matrix
[1 0]
[0 1]
Domain: Vector space of dimension 2 over Rational Field
Codomain: Ambient free module of rank 2 over the principal ideal domain Integer Ring

Here the codomain should be a Q-vector space, not a Z-module, this is not a morphism.

Component: linear algebra

Author: Thomas Rüd

Branch/Commit: 4143a2f

Reviewer: Vincent Delecroix, Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/31818

toadrush commented 3 years ago

Description changed:

--- 
+++ 
@@ -1 +1,17 @@
+Creating a free module morphism with a matrix and without specifying its codomain, currently creates a codomain spanned by row of the matrix. 

+This causes a problem when creating a vector-space homomorphism with an integer matrix.
+
+
+```
+sage: V = QQ^2; m = identity_matrix(2)                                          
+sage: V.hom(m)                                                                  
+Free module morphism defined by the matrix
+[1 0]
+[0 1]
+Domain: Vector space of dimension 2 over Rational Field
+Codomain: Ambient free module of rank 2 over the principal ideal domain Integer Ring
+```
+
+Here the codomain should be a **Q**-vector space, not a **Z**-module, this is not a morphism.
+ 
toadrush commented 3 years ago

Author: Thomas Rüd

toadrush commented 3 years ago

Branch: u/rud/creating_a_module_homomorphism_with_a_matrix_may_build_the_wrong_codomain

toadrush commented 3 years ago
comment:3

Checking if the fix broke anything somewhere else.


New commits:

443cf6afirst commit, added a pushout, now the example in the description works as intended
toadrush commented 3 years ago

Commit: 443cf6a

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 443cf6a to a209a54

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

a209a54commented the change
videlec commented 3 years ago
comment:5

I don't think this is a good fix. There is no reason to enforce the codomain to be a pushout of the domain (and something else). The construction of a morphism with generators should use whatever im_gens is provided or raise an error.

More importantly, the only problem I see with the description is the fact that the result is not a morphism. This should have been detected and an appropriate error raised.

videlec commented 3 years ago
comment:6

And the problem has few to do with matrices

sage: V = QQ^2
sage: V.hom((ZZ^2).basis())
Free module morphism defined by the matrix
[1 0]
[0 1]
Domain: Vector space of dimension 2 over Rational Field
Codomain: Ambient free module of rank 2 over the principal ideal domain Integer Ring
videlec commented 3 years ago

Reviewer: Vincent Delecroix

videlec commented 3 years ago
comment:7

According to the documentation, the correct code is

sage: V = QQ^2
sage: H = V.Hom(V)
sage: H(identity_matrix(2))
Vector space morphism represented by the matrix:
[1 0]
[0 1]
Domain: Vector space of dimension 2 over Rational Field
Codomain: Vector space of dimension 2 over Rational Field
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

9a66ea5changed the pushout to an error
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from a209a54 to 9a66ea5

toadrush commented 3 years ago
comment:9

When we use "Hom(V)", or equivalently "hom(-, V)", we specify the codomain. It should indeed output an error if the wrong codomain is given, instead of the current

sage: V = QQ^2
sage: V.hom(identity_matrix(2), ZZ^2)                                              
Vector space morphism represented by the matrix:
[1 0]
[0 1]
Domain: Vector space of dimension 2 over Rational Field
Codomain: Ambient free module of rank 2 over the principal ideal domain Integer Ring

The question here should be : given a list of images, what is the correct way to build a codomain?

Mathematically, the pushout is the minimal way to make it work, and won't do anything if the codomain base ring is already big enough. However, I don't have a strong opinion about whether it should always work, so I pushed the suggested change, and added this case as an example in the documentation so that it's clearer that for our example, one should write either

sage: V = QQ^2
sage: V.hom(identity_matrix(2), V)

or

sage: V = QQ^2
sage: V.hom(identity_matrix(QQ, 2))

Now the previous cases return a ValueError

sage: V = QQ^2; W = ZZ^2                                                                                                         
sage: V.hom(identity_matrix(2))                                               
Traceback (most recent call last):
...
ValueError: The base ring of the domain must be contained in the base ring of the codomain

New commits:

9a66ea5changed the pushout to an error
videlec commented 3 years ago
comment:10

Looks already better. In your code you are modifying the Parent class which is the base class for any object in Sage, including sets, additive magmas, etc. In your situation, you want to fix a problem concerning homsets of A-modules. That is sage/modules/free_module_homspace.py.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 9a66ea5 to 67fb789

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

67fb789switched the fix to free modules homspaces
toadrush commented 3 years ago
comment:12

Sounds good. I changed it to the correct homspace, and I only allowed zero maps when no coercion exists.

videlec commented 3 years ago
comment:13

As you might have noticed, this breaks a lot of code (especially because number field ideals). The patchbot reports are available by clicking on the 9.3 ? button below "Status badges" (yellow means doctest failures)

src/sage/rings/function_field/function_field.py  # 19 doctests failed
src/sage/schemes/elliptic_curves/isogeny_small_degree.py  # 22 doctests failed
src/sage/schemes/elliptic_curves/ell_number_field.py  # 207 doctests failed
src/sage/schemes/cyclic_covers/cycliccover_finite_field.py  # 1 doctest failed
src/sage/schemes/elliptic_curves/ell_rational_field.py  # 6 doctests failed
src/sage/rings/valuation/mapped_valuation.py  # 1 doctest failed
src/sage/schemes/elliptic_curves/height.py  # 31 doctests failed
src/sage/rings/function_field/function_field_valuation.py  # 4 doctests failed
src/sage/dynamics/arithmetic_dynamics/projective_ds.py  # 22 doctests failed
src/sage/rings/number_field/number_field.py  # 95 doctests failed
src/sage/schemes/elliptic_curves/kraus.py  # 26 doctests failed
src/sage/schemes/elliptic_curves/isogeny_class.py  # 54 doctests failed
src/sage/schemes/elliptic_curves/heegner.py  # 133 doctests failed
src/sage/arith/misc.py  # 7 doctests failed
src/sage/rings/valuation/augmented_valuation.py  # 10 doctests failed
src/sage/schemes/elliptic_curves/gal_reps_number_field.py  # 52 doctests failed
src/sage/modular/modform_hecketriangle/hecke_triangle_group_element.py  # 595 doctests failed
src/sage/modular/abvar/lseries.py  # 5 doctests failed
src/sage/modular/modform_hecketriangle/graded_ring_element.py  # 303 doctests failed
src/sage/modules/free_module_integer.py  # 7 doctests failed
src/sage/modular/modform/find_generators.py  # 3 doctests failed
src/sage/schemes/riemann_surfaces/riemann_surface.py  # 1 doctest failed
src/sage/schemes/elliptic_curves/saturation.py  # 18 doctests failed
src/sage/modules/torsion_quadratic_module.py  # 87 doctests failed
src/sage/modules/free_module.py  # 49 doctests failed
src/sage/groups/fqf_orthogonal.py  # 69 doctests failed
src/sage/rings/tests.py  # 2 doctests failed
src/sage/modular/modform_hecketriangle/readme.py  # 278 doctests failed
src/sage/structure/element.pyx  # 6 doctests failed
src/sage/quadratic_forms/genera/genus.py  # 9 doctests failed
src/sage/schemes/elliptic_curves/ell_curve_isogeny.py  # 7 doctests failed
src/sage/schemes/elliptic_curves/ell_finite_field.py  # Timed out (and interrupt failed)
src/sage/matrix/matrix2.pyx  # 59 doctests failed
src/sage/doctest/forker.py  # 1 doctest failed
src/sage/matrix/matrix_integer_dense.pyx  # 6 doctests failed
src/sage/modular/modform_hecketriangle/abstract_space.py  # 278 doctests failed
src/sage/algebras/lie_algebras/nilpotent_lie_algebra.py  # 1 doctest failed
src/sage/rings/polynomial/polynomial_element.pyx  # 13 doctests failed
src/sage/modular/modsym/ambient.py  # 2 doctests failed
src/sage/geometry/cone.py  # 24 doctests failed
src/sage/rings/function_field/ideal.py  # 80 doctests failed
src/sage/rings/number_field/totallyreal_rel.py  # 23 doctests failed
src/sage/rings/number_field/bdd_height.py  # 5 doctests failed
src/sage/rings/finite_rings/finite_field_base.pyx  # 2 doctests failed
src/sage/schemes/toric/chow_group.py  # 146 doctests failed
src/sage/modular/abvar/abvar.py  # 167 doctests failed
src/sage/misc/functional.py  # 5 doctests failed
src/sage/schemes/projective/projective_rational_point.py  # 1 doctest failed
src/sage/modular/local_comp/local_comp.py  # 8 doctests failed
src/sage/algebras/quatalg/quaternion_algebra.py  # 5 doctests failed
src/sage/groups/generic.py  # 8 doctests failed
src/sage/modular/modform_hecketriangle/space.py  # 139 doctests failed
src/sage/modules/free_quadratic_module_integer_symmetric.py  # 88 doctests failed
src/sage/categories/pushout.py  # 20 doctests failed
src/sage/schemes/elliptic_curves/Qcurves.py  # 9 doctests failed
src/sage/modular/abvar/homspace.py  # 49 doctests failed
src/sage/modular/modform_hecketriangle/element.py  # 40 doctests failed
src/sage/schemes/hyperelliptic_curves/hyperelliptic_generic.py  # 5 doctests failed
src/sage/combinat/permutation.py  # 2 doctests failed
src/sage/modular/modsym/space.py  # 21 doctests failed
src/sage/schemes/elliptic_curves/ell_point.py  # 45 doctests failed
src/sage/rings/valuation/valuation.py  # 2 doctests failed
src/sage/schemes/toric/variety.py  # 16 doctests failed
src/sage/schemes/elliptic_curves/BSD.py  # 2 doctests failed
src/sage/schemes/toric/divisor.py  # 2 doctests failed
src/sage/modular/modform_hecketriangle/hecke_triangle_groups.py  # 147 doctests failed
src/sage/modular/modform_hecketriangle/subspace.py  # 69 doctests failed
src/sage/rings/number_field/S_unit_solver.py  # 17 doctests failed
src/sage/schemes/toric/library.py  # 3 doctests failed
src/sage/schemes/elliptic_curves/cm.py  # 2 doctests failed
src/sage/modular/hecke/submodule.py  # 1 doctest failed
src/sage/schemes/toric/morphism.py  # 69 doctests failed
src/sage/schemes/elliptic_curves/ell_field.py  # 9 doctests failed
src/sage/modular/quatalg/brandt.py  # 4 doctests failed
src/sage/rings/function_field/maps.py  # 9 doctests failed
src/sage/modular/modform_hecketriangle/abstract_ring.py  # 277 doctests failed
src/sage/geometry/toric_lattice.py  # 5 doctests failed
src/sage/rings/polynomial/multi_polynomial.pyx  # 2 doctests failed
src/sage/combinat/words/finite_word.py  # 3 doctests failed
src/sage/rings/number_field/number_field_element_quadratic.pyx  # 42 doctests failed
src/sage/rings/number_field/number_field_rel.py  # 11 doctests failed
src/sage/rings/number_field/galois_group.py  # 5 doctests failed
src/sage/rings/function_field/order.py  # 50 doctests failed
src/sage/algebras/lie_algebras/subalgebra.py  # 20 doctests failed
src/sage/schemes/generic/algebraic_scheme.py  # 5 doctests failed
src/sage/geometry/fan.py  # 11 doctests failed
src/sage/rings/number_field/order.py  # 343 doctests failed
src/sage/modular/arithgroup/congroup_gammaH.py  # 5 doctests failed
src/sage/modular/abvar/torsion_subgroup.py  # 11 doctests failed
src/sage/geometry/fan_morphism.py  # 58 doctests failed
src/sage/rings/complex_mpfr.pyx  # 1 doctest failed
src/sage/modular/local_comp/smoothchar.py  # 44 doctests failed
src/sage/schemes/plane_conics/con_number_field.py  # 10 doctests failed
src/sage/rings/number_field/number_field_ideal.py  # 79 doctests failed
src/doc/en/thematic_tutorials/explicit_methods_in_number_theory/nf_galois_groups.rst  # 1 doctest failed
src/sage/combinat/words/morphism.py  # 25 doctests failed
src/sage/matrix/matrix_modn_sparse.pyx  # 1 doctest failed
src/sage/rings/finite_rings/residue_field.pyx  # 257 doctests failed
src/sage/categories/rings.py  # 3 doctests failed
src/sage/rings/integer_ring.pyx  # 7 doctests failed
src/sage/schemes/projective/projective_morphism.py  # 9 doctests failed
src/sage/rings/padics/padic_valuation.py  # 22 doctests failed
src/sage/schemes/projective/projective_point.py  # 13 doctests failed
src/sage/combinat/words/word_generators.py  # 4 doctests failed
src/sage/rings/finite_rings/integer_mod_ring.py  # 2 doctests failed
src/sage/modular/cusps_nf.py  # 111 doctests failed
src/sage/modules/fg_pid/fgp_module.py  # 265 doctests failed
src/sage/quivers/representation.py  # 1 doctest failed
src/sage/groups/abelian_gps/abelian_group.py  # 4 doctests failed
src/sage/modular/abvar/finite_subgroup.py  # 106 doctests failed
src/sage/rings/valuation/limit_valuation.py  # 3 doctests failed
src/sage/modular/dirichlet.py  # 11 doctests failed
src/sage/schemes/toric/points.py  # 32 doctests failed
src/sage/schemes/elliptic_curves/ell_torsion.py  # 16 doctests failed
src/sage/rings/ring_extension.pyx  # 54 doctests failed
src/sage/modular/modsym/p1list_nf.py  # 144 doctests failed
src/sage/rings/ring.pyx  # 18 doctests failed
src/sage/rings/polynomial/multi_polynomial_libsingular.pyx  # 2 doctests failed
src/sage/rings/morphism.pyx  # 6 doctests failed
src/sage/modular/modform_hecketriangle/constructor.py  # 12 doctests failed
src/sage/algebras/lie_algebras/morphism.py  # 1 doctest failed
src/sage/schemes/elliptic_curves/cardinality.py  # 4 doctests failed
src/sage/modular/abvar/morphism.py  # 62 doctests failed
src/sage/geometry/toric_lattice_element.pyx  # 2 doctests failed
src/sage/categories/algebra_functor.py  # 5 doctests failed
src/sage/rings/polynomial/omega.py  # 5 doctests failed
src/doc/en/constructions/number_fields.rst  # 2 doctests failed
src/sage/rings/number_field/number_field_base.pyx  # 3 doctests failed
src/sage/matrix/matrix_integer_sparse.pyx  # 1 doctest failed
src/sage/geometry/fan_isomorphism.py  # 4 doctests failed
src/sage/tests/books/judson-abstract-algebra/domains-sage.py  # 5 doctests failed
src/sage/algebras/lie_algebras/lie_algebra_element.pyx  # 24 doctests failed
src/sage/categories/homset.py  # 2 doctests failed
src/sage/categories/quotient_fields.py  # 19 doctests failed
src/sage/rings/ring_extension_element.pyx  # 25 doctests failed
src/sage/schemes/elliptic_curves/ell_local_data.py  # 25 doctests failed
src/sage/schemes/projective/projective_space.py  # 1 doctest failed
src/sage/structure/formal_sum.py  # 1 doctest failed
src/sage/tests/book_stein_modform.py  # 1 doctest failed
src/sage/groups/abelian_gps/abelian_aut.py  # 3 doctests failed
src/sage/modular/abvar/torsion_point.py  # 30 doctests failed
src/sage/modular/arithgroup/congroup_gamma0.py  # 1 doctest failed
src/sage/modular/abvar/abvar_ambient_jacobian.py  # 6 doctests failed
src/doc/en/thematic_tutorials/explicit_methods_in_number_theory/modabvar.rst  # 6 doctests failed
src/sage/modular/abvar/homology.py  # 24 doctests failed
src/sage/schemes/generic/morphism.py  # 4 doctests failed
src/sage/schemes/toric/homset.py  # 7 doctests failed
src/sage/tests/books/judson-abstract-algebra/cyclic-sage.py  # 11 doctests failed
src/sage/schemes/affine/affine_space.py  # 1 doctest failed
src/sage/modular/abvar/abvar_newform.py  # 3 doctests failed
src/sage/rings/number_field/number_field_ideal_rel.py  # 2 doctests failed
src/sage/modular/abvar/cuspidal_subgroup.py  # 43 doctests failed
src/sage/schemes/product_projective/space.py  # 1 doctest failed
src/sage/groups/matrix_gps/isometries.py  # 13 doctests failed
src/sage/homology/chain_complex.py  # 1 doctest failed
src/sage/categories/principal_ideal_domains.py  # 5 doctests failed
src/sage/schemes/product_projective/morphism.py  # 3 doctests failed
src/sage/rings/number_field/class_group.py  # 30 doctests failed
src/sage/modules/fg_pid/fgp_morphism.py  # 113 doctests failed
src/sage/modular/modform_hecketriangle/functors.py  # 87 doctests failed
src/sage/rings/fraction_field.py  # 8 doctests failed
src/sage/modules/free_module_morphism.py  # 28 doctests failed
src/sage/rings/padics/padic_extension_generic.py  # 3 doctests failed
src/sage/quivers/path_semigroup.py  # 2 doctests failed
src/sage/groups/additive_abelian/additive_abelian_group.py  # 9 doctests failed
src/sage/modules/matrix_morphism.py  # 18 doctests failed
src/sage/rings/valuation/gauss_valuation.py  # 1 doctest failed
src/sage/quadratic_forms/quadratic_form.py  # 3 doctests failed
src/sage/modules/free_quadratic_module.py  # 2 doctests failed
src/sage/modular/modform_hecketriangle/series_constructor.py  # 39 doctests failed
src/sage/rings/finite_rings/finite_field_prime_modn.py  # 2 doctests failed
src/sage/structure/category_object.pyx  # 7 doctests failed
src/sage/modular/modform_hecketriangle/graded_ring.py  # 50 doctests failed
src/sage/schemes/cyclic_covers/charpoly_frobenius.py  # 1 doctest failed
src/sage/modular/modform/l_series_gross_zagier_coeffs.pyx  # 2 doctests failed
src/sage/modules/fg_pid/fgp_element.py  # 108 doctests failed
src/sage/rings/valuation/scaled_valuation.py  # 1 doctest failed
src/doc/en/thematic_tutorials/explicit_methods_in_number_theory/nf_orders.rst  # 10 doctests failed
src/doc/de/tutorial/tour_numtheory.rst  # 1 doctest failed
src/sage/rings/complex_interval_field.py  # 1 doctest failed
src/doc/es/tutorial/tour_numtheory.rst  # 1 doctest failed
src/doc/ja/tutorial/tour_numtheory.rst  # 1 doctest failed
src/doc/fr/tutorial/tour_numtheory.rst  # 1 doctest failed
src/doc/en/tutorial/tour_numtheory.rst  # 1 doctest failed
src/doc/pt/tutorial/tour_numtheory.rst  # 1 doctest failed
src/doc/ru/tutorial/tour_numtheory.rst  # 1 doctest failed
src/sage/geometry/hyperplane_arrangement/affine_subspace.py  # 13 doctests failed
src/sage/modules/module_functors.py  # 4 doctests failed
src/sage/categories/function_fields.py  # 1 doctest failed
src/sage/modules/free_module_homspace.py  # 6 doctests failed
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 67fb789 to 25c62c1

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

25c62c1fixed bugs
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 25c62c1 to 4d9fd9a

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

4d9fd9achanged the error type
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

8f17598added a test for the fix
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 4d9fd9a to 8f17598

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

fa796d1fixed a typo
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 8f17598 to fa796d1

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

dd528edcompleted a test
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from fa796d1 to dd528ed

roed314 commented 3 years ago
comment:20

I think this should be okay now, but I'll give Vincent a chance to look since he was reviewing it.

videlec commented 3 years ago
comment:21

Some details

tscrim commented 3 years ago
comment:22

You also need to change

-        integers.
+        integers. ::
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from dd528ed to c08aa84

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

c08aa84fixed typos
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

4143a2ffixed typos
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from c08aa84 to 4143a2f

videlec commented 3 years ago

Changed reviewer from Vincent Delecroix to Vincent Delecroix, Travis Scrimshaw

vbraun commented 3 years ago

Changed branch from u/rud/creating_a_module_homomorphism_with_a_matrix_may_build_the_wrong_codomain to 4143a2f