Opened 6 years ago

Closed 6 years ago

#17865 closed enhancement (duplicate)

get rid of _native_coercion_ranks_inv and _native_coercion_ranks

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: coercion Keywords:
Cc: Merged in:
Authors: Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

In the file sage/structure/coerce.pyx there is an homemade coercion for python numeric objects (i.e. bool -> int -> long -> float -> complex). Instead we should use the ready made python function coerce. Or even use the PyNumber_Coerce function from the C API.

Change History (7)

comment:1 Changed 6 years ago by vdelecroix

  • Branch set to u/vdelecroix/17685
  • Commit set to 1ca0e2be9cd73baba70d378e18405aa306aaad9a
  • Status changed from new to needs_review

Last 10 new commits:

a60134ctrac #17740: review 1 (documentation)
728811dtrac #17740: review 2 (clean Errors)
96c1a03trac #17740: review 3 (less in try/except block)
2cb51c0Re-introduce action of fraction field as fallback for division action.
bff474bBetter _pseudo_fraction_field default implementation.
9c970aetrac #17740: merge sage-6.6.beta1
2075e2etrac #17740: avoid parent deaths
4fcee82Merge commit '2075e2e' into t/17800/ticket/17800
7083f19Remove use of PY_IS_NUMERIC
1ca0e2btrac #17685: native coercions with PyNumber_Coerce

comment:2 Changed 6 years ago by vdelecroix

The new solution looks twice faster from Python

Old timings:

sage: cm = sage.structure.element.get_coercion_model()
sage: %timeit cm.canonical_coercion(1r,2.r)
1000000 loops, best of 3: 520 ns per loop
sage: %timeit cm.canonical_coercion(1r,complex(1r,1r))
1000000 loops, best of 3: 812 ns per loop

New timings

sage: cm = sage.structure.element.get_coercion_model()
sage: %timeit cm.canonical_coercion(1r,2.r)
1000000 loops, best of 3: 234 ns per loop
sage: %timeit cm.canonical_coercion(1r,complex(1r,1r))
1000000 loops, best of 3: 469 ns per loop

comment:3 Changed 6 years ago by vdelecroix

  • Authors set to Vincent Delecroix

comment:4 Changed 6 years ago by vdelecroix

  • Status changed from needs_review to needs_work

conflicts with #17862 and there is a reference counting issue...

comment:5 Changed 6 years ago by vdelecroix

  • Authors Vincent Delecroix deleted
  • Branch u/vdelecroix/17685 deleted
  • Commit 1ca0e2be9cd73baba70d378e18405aa306aaad9a deleted
  • Dependencies #17862 deleted
  • Milestone changed from sage-6.6 to sage-duplicate/invalid/wontfix
  • Status changed from needs_work to needs_review

Hello,

I propose to close this one as duplicate because I found a much better solution in #18076.

Vincent

comment:6 Changed 6 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

comment:7 Changed 6 years ago by vbraun

  • Resolution set to duplicate
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.