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

performance bug in subs of fraction field #37122

Closed
2 tasks done
mantepse opened this issue Jan 21, 2024 · 0 comments · Fixed by #37143
Closed
2 tasks done

performance bug in subs of fraction field #37122

mantepse opened this issue Jan 21, 2024 · 0 comments · Fixed by #37143

Comments

@mantepse
Copy link
Contributor

Steps To Reproduce

Substitutions in the fraction field of a polynomial ring are much slower than necessary:

sage: P = PolynomialRing(QQ, ["x%s" % i for i in range(1000)])
sage: PF = P.fraction_field()
sage: p = sum(i*P.gen(i) for i in range(50,52)) / sum(i*P.gen(i) for i in range(51,53)); v = P.gen(51)
sage: %time p.subs({v: 100})
CPU times: user 55.4 ms, sys: 0 ns, total: 55.4 ms
Wall time: 55.4 ms
(50*x50 + 5100)/(52*x52 + 5100)
sage: %time p.numerator().subs({v: 100}) / p.denominator().subs({v: 100})
CPU times: user 3.32 ms, sys: 0 ns, total: 3.32 ms
Wall time: 3.33 ms
(50*x50 + 5100)/(52*x52 + 5100)

Expected Behavior

There should be not much difference in timing.

Actual Behavior

The method called is Element.subs, which, first of all, creates a dictionary containing all the gens of the parent.

Additional Information

#1381 seems to be orthogonal to this.

Environment

irrelevant.

Checklist

  • I have searched the existing issues for a bug report that matches the one I want to file, without success.
  • I have read the documentation and troubleshoot guide
vbraun pushed a commit to vbraun/sage that referenced this issue Feb 7, 2024
sagemathgh-37143: special subs for fractions
    
We create a specialised method `subs` for `FractionFieldElements`, for
better performance.  In particular, the generic `Element.subs` method
insists on replacing all gens, which is a problem in polynomial rings
with many variables, such as (potentially) the `InfinitePolynomialRing`.

Fixes sagemath#37122.
    
URL: sagemath#37143
Reported by: Martin Rubey
Reviewer(s): Martin Rubey, Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue Feb 11, 2024
sagemathgh-37143: special subs for fractions
    
We create a specialised method `subs` for `FractionFieldElements`, for
better performance.  In particular, the generic `Element.subs` method
insists on replacing all gens, which is a problem in polynomial rings
with many variables, such as (potentially) the `InfinitePolynomialRing`.

Fixes sagemath#37122.
    
URL: sagemath#37143
Reported by: Martin Rubey
Reviewer(s): Martin Rubey, Travis Scrimshaw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant