Division of modules by basering elements should not pass to the fraction field.

[robertwb]

sage: cm = sage.structure.element.get_coercion_model()
sage: cm.bin_op(GF(5)['x'].gen(), 7, operator.div).parent()
Fraction Field of Univariate Polynomial Ring in x over Finite Field of size 5

The problem is that we are not finding the inverse because we are passing to the fraction field of ZZ before coercing to GF(5).

[nbruin]

Can we also address here why asking the coercion system to do this operation does not lead to the same result as when we just do the computation?

sage: parent(GF(5)['x'].gen()/7)
Univariate Polynomial Ring in x over Finite Field of size 5

There may be good reasons for this (performance?) so I'm not immediately suggesting that we change the code to use the same path in both cases, but we should document that and why we're not.

---

New commits:

​6a507b7

Better inversion of module actions for correct division coercions.

[robertwb]

It was probably added because coercion simply didn't used to do the right thing. I've filed #17741 about removing the redundant div which may uncover surprises.

[vdelecroix]

My main concern is that your solution ignore iteration

sage: cm.get_action(GF(5)['x']['y'], ZZ, operator.div)
Right inverse action by Fraction Field of Univariate Polynomial Ring in x over Finite Field of size 5 on Univariate Polynomial Ring in y over Univariate Polynomial Ring in x over Finite Field of size 5
with precomposition on right by Composite map:
  From: Integer Ring
  To:   Fraction Field of Univariate Polynomial Ring in x over Finite Field of size 5
  Defn:   Natural morphism:
          From: Integer Ring
          To:   Finite Field of size 5
        then
          Polynomial base injection morphism:
          From: Finite Field of size 5
          To:   Univariate Polynomial Ring in x over Finite Field of size 5
        then
          Ring Coercion morphism:
          From: Univariate Polynomial Ring in x over Finite Field of size 5
          To:   Fraction Field of Univariate Polynomial Ring in x over Finite Field of size 5

I think that GF(5)['x,y'] and GF(5)['x']['y'] should behave similarly.

Vincent

[robertwb]

Yes, I've actually been thinking about that as well, and I'm not sure exactly how best to handle this. GF(5)x?y? is a GF(5)-module but represented as a GF(5)x?-module. Or, put otherwise, (most) elements of ZZ have an inverse in GF(5); we want to take the most conservative inverse possible.

I used to be very against partial coercions like QQ -> GF(p), but I'm not as sure anymore.

[robertwb]

Note that this behavior is not new

sage: parent(GF(5)['x']['y'].gen() / 2)
Univariate Polynomial Ring in y over Fraction Field of Univariate Polynomial Ring in x over Finite Field of size 5

More generally, given an R-module M and an element in R0 such that R0 -> R1 -> ... -> Rn -> R, where along this chain should the multiplicative inverse be calculated? Maybe the first (last?) Ri that is already field? Starts to feel a bit ad-hoc...

[git]

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

​7744207	Better behavior for polynomial division.
​1e88c72	Another polynomial division action test.

[robertwb]

OK, I've implemented the above behavior.

[vdelecroix]

I wrote trivial commits while I was reading the code. One thing to mention: in the documentation you should use :trac:`17440` instead of #17440.

One last question: in coerce.pyx, you wrote

try:
    return ~right_mul
except TypeError: # action may not be invertible
    self._record_exception()

Shouldn't we catch only CoercionException?

---

New commits:

​a60134c	trac #17740: review 1 (documentation)
​728811d	trac #17740: review 2 (clean Errors)
​96c1a03	trac #17740: review 3 (less in try/except block)

[vdelecroix]

And something is broken:

sage -t schemes/toric/divisor.py
**********************************************************************
File "schemes/toric/divisor.py", line 35, in sage.schemes.toric.divisor
Failed example:
    (1/2)*Dx + Dy/3 - Dz
Exception raised:
    Traceback (most recent call last):
    ...
    TypeError: unsupported operand parent(s) for '/': 'Group of toric ZZ-Weil divisors on 2-d CPR-Fano toric variety covered by 6 affine patches' and 'Integer Ring'
**********************************************************************
File "schemes/toric/divisor.py", line 40, in sage.schemes.toric.divisor
Failed example:
    (Dx/2).parent()
Exception raised:
    Traceback (most recent call last):
    ...
    TypeError: unsupported operand parent(s) for '/': 'Group of toric ZZ-Weil divisors on 2-d CPR-Fano toric variety covered by 6 affine patches' and 'Integer Ring'
**********************************************************************
File "schemes/toric/divisor.py", line 866, in sage.schemes.toric.divisor.ToricDivisor_generic.is_Weil
Failed example:
    (D/2).is_Weil()
Exception raised:
    Traceback (most recent call last):
    ...
    TypeError: unsupported operand parent(s) for '/': 'Group of toric ZZ-Weil divisors on 2-d CPR-Fano toric variety covered by 3 affine patches' and 'Integer Ring'
**********************************************************************
File "schemes/toric/divisor.py", line 893, in sage.schemes.toric.divisor.ToricDivisor_generic.is_QQ_Weil
Failed example:
    (D/2).is_QQ_Weil()
Exception raised:
    Traceback (most recent call last):
    ...
    TypeError: unsupported operand parent(s) for '/': 'Group of toric ZZ-Weil divisors on 2-d CPR-Fano toric variety covered by 3 affine patches' and 'Integer Ring'
**********************************************************************
3 items had failures:
   2 of  42 in sage.schemes.toric.divisor
   1 of   5 in sage.schemes.toric.divisor.ToricDivisor_generic.is_QQ_Weil
   1 of   5 in sage.schemes.toric.divisor.ToricDivisor_generic.is_Weil
    [363 tests, 4 failures, 2.53 s]
----------------------------------------------------------------------
sage -t schemes/toric/divisor.py  # 4 doctests failed
----------------------------------------------------------------------

[vdelecroix]

Replying to vdelecroix:

And something is broken:

sage -t schemes/toric/divisor.py

The problematic line is module_action = type(self)(K, self.codomain()) in coerce_action.pyx

sage: dP6 = toric_varieties.dP6()
sage: Dx,Du,Dy,Dv,Dz,Dw = dP6.toric_divisor_group().gens()
sage: A = RightModuleAction(ZZ, Dx.parent())
sage: ~A
Traceback (most recent call last):
...
    472             print "self.codomain = {}".format(self.codomain())
--> 473             module_action = type(self)(K, self.codomain())
    474             connecting = K.coerce_map_from(self.G)
...
CoercionException: No common base

[robertwb]

Fixed these failing tests.

Also, no, we don't want to only catch CoercionException there.

---

New commits:

​2cb51c0

Re-introduce action of fraction field as fallback for division action.

[vdelecroix]

Still some failing doctests... (the last one only concerns the more recent 6-5.rc0)

File rings/quotient_ring_element.py

line 360, in sage.rings.quotient_ring_element.QuotientRingElement._div_
Failed example:
    1/cuberoot
Exception raised:
    Traceback (most recent call last):
    ...
    TypeError: self must be an integral domain.
**********************************************************************
line 362, in sage.rings.quotient_ring_element.QuotientRingElement._div_
Failed example:
    1/a
Exception raised:
    Traceback (most recent call last):
    ...
    TypeError: self must be an integral domain.
**********************************************************************
File "src/sage/rings/quotient_ring_element.py", line 370, in sage.rings.quotient_ring_element.QuotientRingElement._div_
Failed example:
    1 / S(x1 + x2)
Expected:
    Traceback (most recent call last):
    ...
    ArithmeticError: Division failed. The numerator is not a multiple of the denominator.
Got:
    Traceback (most recent call last):
    ...
    TypeError: self must be an integral domain.

File rings/padics/local_generic_element.pyx

line 63, in sage.rings.padics.local_generic_element.LocalGenericElement._div_
Failed example:
    R = Zp(7, 4, 'fixed-mod'); 1/R(7)
Expected:
    Traceback (most recent call last):
    ...
    ValueError: cannot invert non-unit
Got:
    Traceback (most recent call last):
    ...
    TypeError: This implementation of the p-adic ring does not support fields of fractions.

File rings/padics/padic_ZZ_pX_FM_element.pyx

line 88, in sage.rings.padics.padic_ZZ_pX_FM_element
Failed example:
    1/a
Exception raised:
    Traceback (most recent call last):
    ...
    TypeError: This implementation of the p-adic ring does not support fields of fractions.
**********************************************************************
line 875, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement._div_
Failed example:
    1 / W(14) == ~W(14)
Exception raised:
    Traceback (most recent call last):
    ...
    TypeError: This implementation of the p-adic ring does not support fields of fractions.
**********************************************************************
line 877, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement._div_
Failed example:
    1 / w
Expected:
    Traceback (most recent call last):
    ...
    ValueError: cannot invert non-unit
Got:
    Traceback (most recent call last):
    ...
    TypeError: This implementation of the p-adic ring does not support fields of fractions.
**********************************************************************
line 885, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement._div_
Failed example:
    1/(t+t^2)
Exception raised:
    Traceback (most recent call last):
    ...
    TypeError: This implementation of the p-adic ring does not support fields of fraction.

In modular/modform_hecketriangle/graded_ring_element.py:

sage -t --long src/sage/modular/modform_hecketriangle/graded_ring_element.py
**********************************************************************
line 887, in sage.modular.modform_hecketriangle.graded_ring_element.FormsRingElement._div_
Failed example:
    ((E4.as_ring_element())^(-2)).parent() == (E4^(-2)).parent()
Exception raised:
    Traceback (most recent call last):
    ...
    NotImplementedError: No way to prove that ModularFormsRing(n=8) over Integer Ring is an integral domain!
**********************************************************************
line 889, in sage.modular.modform_hecketriangle.graded_ring_element.FormsRingElement._div_
Failed example:
    (MR(x)^(-3)).parent()
Exception raised:
    Traceback (most recent call last):
    ...
    NotImplementedError: No way to prove that QuasiMeromorphicModularFormsRing(n=8) over Integer Ring is an integral domain!

[vdelecroix]

Hello Robert,

Here is a one-line commit that fixes all doctest issues. I am wondering why these did not appear in the first time... Tell me what you think.

Vincent

---

New commits:

​16ca39d	trac #17440: merge 6.5.rc3
​d1fc166	trac #17440: better _pseudo_fraction_field

[robertwb]

I don't think self is the right thing to return by default, but here's a similar fix.

---

New commits:

​bff474b

Better _pseudo_fraction_field default implementation.

[vdelecroix]

Now, I am not sure anymore that your solution is better. The implementation of division_parent tries to execute ~parent.one(). But the default implementation of __invert__ of an element x in sage.structure.element is precisely to call 1/x (and soonly x.parent().one()/x with #17692). Hence using division_parent should lead to some infinite recursions that we curiously do not see here. No?

[robertwb]

Yes, indeed this should go in with #17692 to prevent possible infinite recursion. Thanks for the review!

[vbraun]

Merge conflict, possibly #17694

[vdelecroix]

Hi Robert,

There was only a trivial conflict in sage/rings/ring.pyx. I did merge with sage-6.6.beta0 in my last commit. I let you set the positive review if this is fine.

Vincent

---

New commits:

​52cc62d

trac #17740: merge sage-6.6.beta0

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​9c970ae

trac #17740: merge sage-6.6.beta1

[vdelecroix]

All long tests pass. Put back in positive review.

[vbraun]

I'm getting this on OSX:

sage -t --long src/sage/structure/coerce_actions.pyx
**********************************************************************
File "src/sage/structure/coerce_actions.pyx", line 419, in sage.structure.coerce_actions.ModuleAction.__invert__
Failed example:
    A = ~RightModuleAction(ZZ, GF(5)['x']); A
Exception raised:
    Traceback (most recent call last):
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 488, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 850, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.structure.coerce_actions.ModuleAction.__invert__[10]>", line 1, in <module>
        A = ~RightModuleAction(ZZ, GF(Integer(5))['x']); A
      File "sage/structure/coerce_actions.pyx", line 464, in sage.structure.coerce_actions.ModuleAction.__invert__ (build/cythonized/sage/structure/coerce_actions.c:7385)
        module_action = type(self)(K, self.codomain())
      File "sage/structure/coerce_actions.pyx", line 376, in sage.structure.coerce_actions.ModuleAction.codomain (build/cythonized/sage/structure/coerce_actions.c:6711)
        return self.underlying_set()
      File "sage/categories/action.pyx", line 186, in sage.categories.action.Action.underlying_set (build/cythonized/sage/categories/action.c:4087)
        raise RuntimeError, "This action acted on a set that became garbage collected"
    RuntimeError: This action acted on a set that became garbage collected
**********************************************************************
File "src/sage/structure/coerce_actions.pyx", line 424, in sage.structure.coerce_actions.ModuleAction.__invert__
Failed example:
    A(x, 2)
Expected:
    3*x
Got:
    1/2*x
**********************************************************************
1 item had failures:
   2 of  15 in sage.structure.coerce_actions.ModuleAction.__invert__
    [90 tests, 2 failures, 0.47 s]

In all these doctests you need to hold domain/codomain explicitly alive.

[git]

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

​2075e2e

trac #17740: avoid parent deaths

[vdelecroix]

Right. We should not kill our parents ;-)

It is strange that I did not get these in the first place... All doctest pass.

Vincent

[robertwb]

Thanks for following up on this. Looks good to me.