Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

combinat/free module tensor construction #37451

Merged

Conversation

mantepse
Copy link
Collaborator

@mantepse mantepse commented Feb 24, 2024

This branch allows to change the base ring of a tensor product of free modules:

            sage: F = CombinatorialFreeModule(ZZ, ['a','b','c']); F
            sage: T = F.tensor(F); T
            Free module generated by {'a', 'b', 'c'} over Integer Ring # Free module generated by {'a', 'b', 'c'} over Integer Ring
            sage: T.change_ring(QQ)
            Free module generated by {'a', 'b', 'c'} over Rational Field # Free module generated by {'a', 'b', 'c'} over Rational Field

@tscrim
Copy link
Collaborator

tscrim commented Feb 25, 2024

Should we also do the same for the cartesian_product construction as well?

@tscrim
Copy link
Collaborator

tscrim commented Apr 8, 2024

LGTM, but what about doing cartesian_product as well?

@mantepse
Copy link
Collaborator Author

mantepse commented Apr 8, 2024

LGTM, but what about doing cartesian_product as well?

Like so?

diff --git a/src/sage/combinat/free_module.py b/src/sage/combinat/free_module.py
index 2f153396fc0..5daabd153fb 100644
--- a/src/sage/combinat/free_module.py
+++ b/src/sage/combinat/free_module.py
@@ -505,6 +505,15 @@ class CombinatorialFreeModule(UniqueRepresentation, Module, IndexedGenerators):
             sage: T.change_ring(QQ)
             Free module generated by {'a', 'b', 'c'} over Rational Field
              # Free module generated by {'a', 'b', 'c'} over Rational Field
+
+            sage: G = CombinatorialFreeModule(ZZ, ['x','y']); G
+            Free module generated by {'x', 'y'} over Integer Ring
+            sage: T = F.cartesian_product(G); T
+            Free module generated by {'a', 'b', 'c'} over Integer Ring
+             (+) Free module generated by {'x', 'y'} over Integer Ring
+            sage: T.change_ring(QQ)
+            Free module generated by {'a', 'b', 'c'} over Rational Field
+             (+) Free module generated by {'x', 'y'} over Rational Field
         """
         if R is self.base_ring():
             return self
@@ -515,7 +524,8 @@ class CombinatorialFreeModule(UniqueRepresentation, Module, IndexedGenerators):
             if isinstance(functor, VectorFunctor):
                 return functor(R)
             from sage.categories.tensor import TensorProductFunctor
-            if isinstance(functor, TensorProductFunctor):
+            from sage.categories.cartesian_product import CartesianProductFunctor
+            if isinstance(functor, (TensorProductFunctor, CartesianProductFunctor)):
                 return functor([f.change_ring(R) for f in args])
         raise NotImplementedError('the method change_ring() has not yet been implemented')

@tscrim
Copy link
Collaborator

tscrim commented Apr 9, 2024

Yes, that's correct.

@mantepse
Copy link
Collaborator Author

mantepse commented Apr 9, 2024

ready

Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you; once a green bot, then you can set a positive review.

Be aware that the extra large PEP8 stuff might result in a merge conflict. I think there is nothing that will conflict now, but I am not 100% certain.

Copy link

github-actions bot commented Apr 9, 2024

Documentation preview for this PR (built with commit c8e1693; changes) is ready! 🎉

@vbraun vbraun merged commit d8742b4 into sagemath:develop Apr 12, 2024
14 checks passed
@mkoeppe mkoeppe added this to the sage-10.4 milestone Apr 12, 2024
@mantepse mantepse deleted the combinat/free_module_tensor_construction branch April 13, 2024 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants