Skip to content

Renames + add deterministic choice of generic linear forms#133

Open
rprebet wants to merge 18 commits into
algebraic-solving:mainfrom
rprebet:fix-curve-param
Open

Renames + add deterministic choice of generic linear forms#133
rprebet wants to merge 18 commits into
algebraic-solving:mainfrom
rprebet:fix-curve-param

Conversation

@rprebet

@rprebet rprebet commented Mar 25, 2026

Copy link
Copy Markdown
Contributor
  • Rename files/functions/types for curve parametrization to avoid confusion with parametric/rational curves. It consists mainly in swapping "rational" and "curve".
  • Bug correction in the computation of zero-dim parametrization of fibers of the curves. This was not replacing non-generic fibers by new ones, as designed first.
  • reverse the order of the parametrization variables : x is the last, and y the second to last. This is to match with the zero-dimensional parametrizsation.
  • If not provided, compute generic linear forms in a deterministic way by always trying the same (infinite) sequence of candidate. The genericity test is based on degree stability and dimension decrease.

@codecov-commenter

codecov-commenter commented Mar 25, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 96.44670% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.92%. Comparing base (91d7230) to head (6ed869a).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/algorithms/curve-param.jl 96.42% 7 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #133      +/-   ##
==========================================
+ Coverage   83.62%   84.92%   +1.30%     
==========================================
  Files          32       33       +1     
  Lines        3188     3357     +169     
==========================================
+ Hits         2666     2851     +185     
+ Misses        522      506      -16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@wegank

wegank commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

Would it be advisable to keep rational_curve_parametrization as a temporary alias, with a deprecation warning saying that the function has been renamed to curve_rational_parametrization, to avoid immediately breaking existing external code?

@mohabsafey

Copy link
Copy Markdown
Collaborator

Very sorry to be slow on this. Since rational_curve_parametrization was quite recent and is not used yet in Oscar, I think we should go for the name change directly (precisely to avoid that this name is used). Otherwise, I am fine with this PR. @ederc , what do you think?

@rprebet rprebet changed the title Renames + bug correction Renames + add deterministic choice of generic linear forms May 26, 2026
@rprebet

rprebet commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

Hi,

I just finished with this PR. I summarized the changes in the first post.
A notable change from user POV is that, contrary to msolve, I now add systematically two generic variables _Z2 and _Z1 to handle all cases as the generic one. This avoids complicated manipulation and several different types output.

@rprebet

rprebet commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

I added genericity tests on the provided linear form to guarantee the function does not output wrong parametrization.

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.

5 participants