Opened 18 months ago

Closed 6 months ago

Last modified 3 days ago

#23835 closed enhancement (fixed)

Replace Maxima with Pynac/Singular in Expression.factor()

Reported by: rws Owned by:
Priority: major Milestone: sage-8.4
Component: symbolics Keywords: performance
Cc: tscrim Merged in:
Authors: Ralf Stephan Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 19cff3a (Commits) Commit:
Dependencies: #23950 Stopgaps:

Description (last modified by rws)

The default for symbolic factorization should be changed. Maxima as default should be replaced with a call to Pynac's factor() implementation, which itself uses Singular at the moment. Maxima should be made available via ex.factor(algorithm=...).

As an example of performance gain here one of the Fateman benchmarks:

sage: var('a b c k s y z')
(a, b, c, k, s, y, z)
sage: f = (1+x+y+z)^20+1
sage: g = (f*(f+1)).expand()
sage: %time _=g.factor()

This takes 11 seconds on 8.1.beta5 and 3.3 seconds with Pynac factor (identical time using polynomial ring).

Change History (13)

comment:1 Changed 17 months ago by rws

  • Dependencies set to #23861

comment:2 Changed 17 months ago by rws

  • Branch set to u/rws/replace_maxima_with_pynac_singular_in_expression_factor__

comment:3 Changed 17 months ago by rws

  • Authors set to Ralf Stephan
  • Commit set to 5157371e406d5686eb9ce21c9a42ffbcde006925
  • Dependencies changed from #23861 to pynac-0.7.12

New commits:

515737123835: Replace Maxima with Pynac/Singular in Expression.factor()

comment:4 Changed 17 months ago by rws

  • Description modified (diff)
  • Keywords performance added

comment:5 Changed 17 months ago by rws

  • Dependencies changed from pynac-0.7.12 to #23950
  • Status changed from new to needs_review

comment:6 Changed 16 months ago by git

  • Commit changed from 5157371e406d5686eb9ce21c9a42ffbcde006925 to e9484ceda20cde440ae5dd9c61cd8346d0a35412

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

6a00170Merge branch 'develop' into t/23835/replace_maxima_with_pynac_singular_in_expression_factor__
e9484ce23835: fix doctest

comment:7 Changed 12 months ago by rws

  • Milestone changed from sage-8.1 to sage-8.2

comment:8 Changed 7 months ago by tscrim

  • Cc tscrim added
  • Milestone changed from sage-8.2 to sage-8.4
  • Reviewers set to Travis Scrimshaw

If you rebase this to 8.4.beta0, I will review it.

comment:9 Changed 6 months ago by tscrim

Ping?

comment:10 Changed 6 months ago by rws

Sorry, due to other work I'm restricting my activities to maintaining Pynac and its upgrade tickets.

comment:11 Changed 6 months ago by tscrim

  • Branch changed from u/rws/replace_maxima_with_pynac_singular_in_expression_factor__ to u/tscrim/expression_factor-23835
  • Commit changed from e9484ceda20cde440ae5dd9c61cd8346d0a35412 to 19cff3a90c5996aad903cf82a90ec31a97b8728f
  • Status changed from needs_review to positive_review

No problem. It ended up being a trivial rebase. So I am treating this ticket as a needs_review.

While I do not see as big of a speed difference (9.5s to 3.2s), it still is quite an improvement. So positive review.


New commits:

19cff3aMerge branch 'u/rws/replace_maxima_with_pynac_singular_in_expression_factor__' of git://trac.sagemath.org/sage into u/tscrim/expression_factor-23835

comment:12 Changed 6 months ago by vbraun

  • Branch changed from u/tscrim/expression_factor-23835 to 19cff3a90c5996aad903cf82a90ec31a97b8728f
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:13 Changed 3 days ago by egourgoulhon

  • Commit 19cff3a90c5996aad903cf82a90ec31a97b8728f deleted

See #27304 for a follow up (bug in factorization of exponentials).

Note: See TracTickets for help on using tickets.