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

enable coercion for tensor constructions #37413

Conversation

mantepse
Copy link
Collaborator

We want to allow acting with an extension of the base ring on a tensor product. For example:

sage: R.<x> = QQ[]
sage: M = CombinatorialFreeModule(QQ, [1,2,3])
sage: m = M.an_element(); m
2*B[1] + 2*B[2] + 3*B[3]
sage: r = x + 1
sage: r * m
(2*x+2)*B[1] + (2*x+2)*B[2] + (3*x+3)*B[3]
sage: T = M.tensor(M)
sage: t = T.an_element(); t
2*B[1] # B[1] + 2*B[1] # B[2] + 3*B[1] # B[3]
sage: r * t
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[19], line 1
----> 1 r * t

File ~/sage-trac/src/sage/structure/element.pyx:1506, in sage.structure.element.Element.__mul__()
   1504     return (<Element>left)._mul_(right)
   1505 if BOTH_ARE_ELEMENT(cl):
-> 1506     return coercion_model.bin_op(left, right, mul)
   1507 
   1508 cdef long value

File ~/sage-trac/src/sage/structure/coerce.pyx:1278, in sage.structure.coerce.CoercionModel.bin_op()
   1276     # We should really include the underlying error.
   1277     # This causes so much headache.
-> 1278     raise bin_op_exception(op, x, y)
   1279 
   1280 cpdef canonical_coercion(self, x, y) noexcept:

TypeError: unsupported operand parent(s) for *: 'Univariate Polynomial Ring in x over Rational Field' and 'Free module generated by {1, 2, 3} over Rational Field # Free module generated by {1, 2, 3} over Rational Field'

Comment on lines +65 to +72
CovariantFunctorialConstruction.__init__(self)
self._forced_category = category
from sage.categories.rings import Rings
if self._forced_category is not None:
codomain = self._forced_category
else:
codomain = Rings()
MultivariateConstructionFunctor.__init__(self, Rings(), codomain)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added this blindly, by copy-pasting from CartesianProductFunctor. Rings() is almost certainly wrong, I have no idea how the correct category is chosen.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The main thing of the _forced_category is to pass it along through the __call__ in the cartesian_product as the construction that is being used might need more conditions (e.g., if something was a ring constructed as a Cartesian product [of rings]). On its own, there isn't any use. Rings() is also too strong as a default category; Modules(base_ring.category()) is likely the best.

@mantepse
Copy link
Collaborator Author

See also #37220.

@mantepse
Copy link
Collaborator Author

Some more info:

sage: R.<x> = QQ[]
sage: M = CombinatorialFreeModule(QQ, [1,2,3])
sage: T = M.tensor(M)
sage: t = T.an_element(); t
2*B[1] # B[1] + 2*B[1] # B[2] + 3*B[1] # B[3]
sage: r = x + 1

sage: from sage.structure.coerce_actions import detect_element_action
sage: cm = sage.structure.element.get_coercion_model()
sage: cm.record_exceptions()

sage: detect_element_action(T, R, False)

sage: print(cm.exception_stack()[0])
Traceback (most recent call last):
  File "sage/structure/coerce_actions.pyx", line 223, in sage.structure.coerce_actions.detect_element_action
    return (RightModuleAction if X_on_left else LeftModuleAction)(Y, X, y, x)
  File "sage/structure/coerce_actions.pyx", line 334, in sage.structure.coerce_actions.ModuleAction.__init__
    self.extended_base = pushout(G, S)
  File "/home/martin/sage-trac/src/sage/categories/pushout.py", line 4531, in pushout
    Z = R_tower[-1][0].common_base(S_tower[-1][0], R_tower[-1][1], S_tower[-1][1])
  File "/home/martin/sage-trac/src/sage/categories/pushout.py", line 380, in common_base
    self._raise_common_base_exception_(
  File "/home/martin/sage-trac/src/sage/categories/pushout.py", line 417, in _raise_common_base_exception_
    raise CoercionException(
sage.structure.coerce_exceptions.CoercionException: No common base ("join") found for FractionField(Integer Ring) and The tensor functorial construction(Free module generated by {1, 2, 3} over Rational Field, Free module generated by {1, 2, 3} over Rational Field).

Copy link

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

@grhkm21
Copy link
Contributor

grhkm21 commented Feb 21, 2024

(I'm not sure if commalg is the correct label, nor do I know anything about this topic)

@mantepse
Copy link
Collaborator Author

I found the following doctest in pushout.py:

            sage: from sage.categories.pushout import pushout
            sage: pushout(QQ, cartesian_product([ZZ]))  # indirect doctest
            Traceback (most recent call last):
            ...
            CoercionException: No common base ("join") found for
            FractionField(Integer Ring) and The cartesian_product functorial construction(Integer Ring).

Unfortunately, there is no comment indicating why we wouldn't want such a coercion.

I tried to override this in TensorProductFunctor, but without luck. For example,

sage: M = CombinatorialFreeModule(ZZ, [1,2,3])
sage: T = tensor([M, M])
sage: t = T.an_element()
sage: 3/2*t

actually calls ConstructionFunctor.common_base, rather than TensorProductFunctor.common_base.

@nbruin
Copy link
Contributor

nbruin commented Feb 22, 2024

sorry, no clue about this code.

@tscrim
Copy link
Collaborator

tscrim commented Feb 23, 2024

It's possible that things are not working because there is no coercion, which is something that is checked.

sage: cartesian_product([ZZ]).has_coerce_map_from(ZZ)
False

We might need to implement a custom _pushout_ method for the Cartesian and tensor product functors to take into account the actions... I am not really sure; it's been a long time since I really tried to figure out the full details about how the pushouts work.

@mantepse
Copy link
Collaborator Author

That's not so good news... I was hoping you know everything :-)

Concerning the __init__, importing Modules there (even locally) yields a circular import (I think), but I am guessing that this is not the most pressing problem.

Maybe there is an alternative for #37033 after all, but I don't see how I can convert some tensor product over R to a tensor product over UndeterminedCoefficientRing(R) before multiplication in unrelated code (or even in Stream_cauchy_mul) happens.

@tscrim
Copy link
Collaborator

tscrim commented Feb 23, 2024

Unfortunately I've done very little with the deep parts of the coercion pushout stuff. I can relearn it, but it has been 5+ years since I last touched it.

Local imports in an __init__ should never yield a circular import unless it is creating something in the module when it is loaded. I guess that is what is happening in this case...? I am not sure if it is needed here anyways.

You likely need a coercion from a module over the smaller base ring to the larger one like this:

sage: V = ZZ^5
sage: W = QQ^5
sage: W.has_coerce_map_from(V)
True

@roed314
Copy link
Contributor

roed314 commented Feb 23, 2024

I'll try to take a look at this over the weekend.

@mantepse
Copy link
Collaborator Author

That would be wonderful! I'll be available most of the time!

@mantepse
Copy link
Collaborator Author

@roed314: I just prepared a very long answer to a reply to Nils Bruin. Doing so I had to check something in my code. This in turn lead me to discover a bug, which might slightly change things. I am too tired right now to fix the bug (it might not be easy), so please give me a night.

This might reduce the problem to nothing, but I don't know yet.

I'm sorry!

Martin

@roed314
Copy link
Contributor

roed314 commented Feb 24, 2024

No problem! This stuff is tricky to get right.

Let me know when you've fixed the problem.

@mantepse
Copy link
Collaborator Author

@roed314, as it turns out, at least the example I was stuck with now works. I simply forgot a change_ring at one specific place. I am not entirely sure yet - because I have too few examples to try - but I hope that this issue is now obsolete.

It will take some time until I have more examples.

Thank you so much for your readiness to help!

@mantepse
Copy link
Collaborator Author

I'll put this branch to sleep. The one little change I actually need is now in #37451.

@mantepse mantepse closed this Feb 24, 2024
@mantepse mantepse deleted the categories/tensor_multivariate_construction branch October 7, 2024 13:28
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.

5 participants