Skip to content

Conversation

@stelip42
Copy link

rank_interpolation_mc is a monte-carlo version of rank_interpolation. Both functions were written for matrices over rational function field / polynomial rings, since in this case llu decomposition is slow.
evaluation_points returns the needed points from the base field for "interpolation".

@thofma thofma self-requested a review December 12, 2025 11:05
@thofma
Copy link
Member

thofma commented Dec 12, 2025

I'll review this.

Copy link
Member

@lgoettgens lgoettgens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave the mathematics to Tommy, but here are some technical things

Copy link
Member

@thofma thofma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, thanks. I left a few comments that should be addressed. There are also a few general things:

  1. The indentation should be consistent. At the moment it is 3 spaces, 4 spaces or 6 spaces.
  2. Make the spaces after the comma consistent. So it should be M[i, j] instead of M[i,j] etc.
  3. The files should have a new line at the end. (We can discuss in person what does means or why your editor does not do this automatically.)
  4. No unicode (aka ϵ).

@stelip42
Copy link
Author

Thanks for reviewing @thofma @lgoettgens. I have applied all your suggestions.

@thofma
Copy link
Member

thofma commented Dec 16, 2025

Needs a few changes. If you are done, you can mark the PR as "ready for review" (there is a button for this near the end of the page).

@stelip42
Copy link
Author

There are still 2 problems @thofma:

  • clog not defined in AbstractAlgebra
  • rank_interpolation and rank_interpolation_mc are part of RationalFunctionField at this moment. This is only a short-term solution.

@thofma
Copy link
Member

thofma commented Dec 17, 2025

Before fixing the clog, please fix the current test and documentation failure.

@thofma thofma marked this pull request as ready for review December 19, 2025 18:35
@thofma thofma marked this pull request as draft December 19, 2025 18:35
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