Skip to content

Add absolute PolyRatFun expansion and fix relative mode - #926

Open
cbmarini wants to merge 1 commit into
form-dev:masterfrom
cbmarini:prf-expand
Open

cbmarini wants to merge 1 commit into
form-dev:masterfrom
cbmarini:prf-expand

Conversation

@cbmarini

Copy link
Copy Markdown
Collaborator

This PR completes support for absolute expansion depths in PolyRatFun and documents the feature. The compiler routine and parts of ExpandRat in ratio.c already contained partial support, but the implementation was incomplete.

The syntax supports positive and negative cutoffs:

PolyRatFun rat(expand,ep,4,fixed);
PolyRatFun rat(expand,ep,-1,fixed);

The previously undocumented relative keyword is also documented. It selects the default expansion mode, so the following two are equivalent:

PolyRatFun rat(expand,ep,4);
PolyRatFun rat(expand,ep,4,relative);

This PR also fixes several expansion paths:

  • Truncate expansions consistently for single-symbol denominators. At relative depth 5, rat(1+ep+...+ep^10,ep) now stops at ep^4, matching the general polynomial-denominator path.
  • Preserve symbols used as numerator coefficients, including cases such as rat(a^3,ep) and rat(a^3,16).
  • Apply fixed cutoffs correctly when multiplying expanded functions across modules.

Questions for review:

  • How should non-symbol numerators such as p1.p2 and single-argument inputs such as rat(a^2) be handled? These inputs currently crash.
  • finddivergence is an alias for the documented divergence mode. Should we keep finddivergence as a keyword?

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage is 66.14% — cbmarini:prf-expand into form-dev:master. No base build found for form-dev:master.

@jodavies

jodavies commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

I think non-symbol content should be forbidden. While it is allowed in gcd_ etc with LocalConvertToPoly, it has not ever been allowed in PolyRatFun arguments, and this is just a special PolyRatFun mode. We should terminate with an error. Actually, I think it would be very useful if for this, and for PolyRatFun in general, the error message actually printed the term with the non-symbol content. It is annoying when you have a termination due to a PolyRatFun argument somewhere within an expression of millions on terms, and you have to work out what exactly is wrong.

If finddivergence was never documented and is just an alias, I would say we should remove it and not over-complicate things with unnecessary extra keywords.

For your second bullet point: are you saying that actually expansion of multivariate rational polynomials is supported? If not, this would also be a very nice feature to have. The code to do it with FLINT could be adapted from here: https://github.com/jodavies/ibppp/blob/main/include/flint_interface.hpp#L311

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants