Ticket #3938 (new defect)

Opened 3 months ago

Last modified 2 months ago

[with patch, needs work] coercion framework converts built-in types to Sage types when it should not

Reported by: cwitty Assigned to: robertwb
Priority: major Milestone: sage-3.2.2
Component: coercion Keywords:
Cc: cwitty

Description (Last modified by mabshoff)

This came up while reviewing #2898, which adds a conversion from float to ZZ (for integral values). After applying that patch, you get: 

sage: 1.0r/8
1/8

That's because of this code in coerce.pyx, which does a conversion rather than a coercion: 

        elif PY_IS_NUMERIC(x):
            try:
                x = yp(x)
                if PY_TYPE_CHECK(yp, type): return x,y

I tried to fix this, but every time I fixed something it broke something else. I'm going to attach my latest non-working patch, which may or may not be a useful place to start.

[Once this ticket has been reviewed and merged #2898 should be closed]

Attachments

trac3938-coercion-converts-native.patch (4.6 kB) - added by cwitty on 08/23/2008 02:04:01 PM.
3938-type-coercion-2.patch (13.7 kB) - added by robertwb on 08/27/2008 09:13:26 AM.
3938-type-coercion-3.patch (7.3 kB) - added by robertwb on 08/27/2008 09:13:58 AM.

Change History

08/23/2008 02:04:01 PM changed by cwitty

  • attachment trac3938-coercion-converts-native.patch added.

08/24/2008 01:43:53 AM changed by robertwb

I've been playing around with this a bit, simplified your patch some, but one consequence is that 

sage: parent(RealField(100)(1.5) + float(1.5)) # good?
<type 'float'>
sage: RealField(100)(2^4000) == float('inf')   # bad?
True

Thoughts?

08/24/2008 03:26:48 PM changed by cwitty

Both of these changes make float act more like RDF. I've sometimes wished that comparison operators would coerce to more-precise types instead of less-precise, but that would be a huge change across big parts of Sage; unless such a policy change is made, I think that both consequences are actually good.

08/27/2008 09:13:26 AM changed by robertwb

  • attachment 3938-type-coercion-2.patch added.

08/27/2008 09:13:58 AM changed by robertwb

  • attachment 3938-type-coercion-3.patch added.

08/27/2008 09:15:59 AM changed by robertwb

  • summary changed from coercion framework converts built-in types to Sage types when it should not to [with patch, needs review] coercion framework converts built-in types to Sage types when it should not.

I feel your pain...what a nasty patch to try and write! Well, I finally feel like I've got a correct, working solution. Apply all three patches.

08/30/2008 04:05:07 PM changed by mabshoff

  • cc set to cwitty.
  • description changed.

Carl,

any chance you could review those three patches?

Cheers,

Michael

09/06/2008 04:13:12 PM changed by anakha

I get 

sage: parent(RealField(10)(1) * float(1))
Real Field with 10 bits of precision

with the patches applied against 3.1.2.alpha4.

09/08/2008 09:27:11 AM changed by robertwb

Is this not the desired behavior?

09/08/2008 10:40:22 AM changed by anakha

  • summary changed from [with patch, needs review] coercion framework converts built-in types to Sage types when it should not to [with patch, positive review] coercion framework converts built-in types to Sage types when it should not.

I thought the goal was to convert to the type with most precision. It seems I was mistaken and this is trying to convert to the type with less precision.

In that case it works and passes a good chunk of the test suite. (I can't test the rest because of the interfaces/lisp.py failure)

It also passes its own tests and the code looks good.

09/18/2008 06:45:18 PM changed by mabshoff

This patch causes a Heisenbug: 

mabshoff@sage:/scratch/mabshoff/release-cycle/sage-3.1.3.alpha0$ ./sage -t -long devel/sage/sage/modular/modsym/ambient.py
sage -t -long devel/sage/sage/modular/modsym/ambient.py     

------------------------------------------------------------
Unhandled SIGSEGV: A segmentation fault occured in SAGE.
This probably occured because a *compiled* component
of SAGE has a bug in it (typically accessing invalid memory)
or is not properly wrapped with _sig_on, _sig_off.
You might want to run SAGE under gdb with 'sage -gdb' to debug this.
SAGE will now terminate (sorry).
------------------------------------------------------------


A mysterious error (perphaps a memory error?) occurred, which may have crashed doctest.
         [4.6 s]
exit code: 768

It does not happen without "-long" and when running "-long -verbose" it also seems to pass. I guess it is time to valgrind :)

Cheers,

Michael

09/18/2008 06:45:37 PM changed by mabshoff

  • summary changed from [with patch, positive review] coercion framework converts built-in types to Sage types when it should not to [with patch, needs work] coercion framework converts built-in types to Sage types when it should not.