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

[Merged by Bors] - feat: improve polyrith by testing for membership in the radical #7790

Closed
wants to merge 15 commits into from

Conversation

robertylewis
Copy link
Member

@robertylewis robertylewis commented Oct 20, 2023

In @hrmacbeth 's tutorial on polyrith, there were examples of problems that it could almost solve, but failed. The goal was not expressible as a linear combination of the hypotheses but a power of the goal was.

example (x y z : ℚ) (h : x = y) (h2 : x * y = 0) : x + y*z = 0 :=
sorry

Mathematically, x+y*z is in the radical of the ideal generated by x-y, x*y. There's a "standard trick" for testing membership in the radical without a search for the proper exponent: see e.g. section 2.2 of arxiv.org/pdf/1007.3615.pdf or 4.2 Prop 8 of Cox, Little, O'Shea.

This PR implements the trick in the Sage call made by polyrith. When the power returned is n > 1, we use linear_combination (exp := n) to check the certificate (#7789 ).

The polyrith test infrastructure still needs to be ported from mathlib3. All tests in the test file succeed when they are uncommented. A future PR will restore the old test suite.

@robertylewis robertylewis added the t-meta Tactics, attributes or user commands label Oct 20, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added blocked-by-other-PR This PR depends on another PR to Mathlib (this label is automatically managed by a bot) and removed blocked-by-other-PR This PR depends on another PR to Mathlib (this label is automatically managed by a bot) labels Oct 20, 2023
@digama0
Copy link
Member

digama0 commented Oct 21, 2023

Note: I have pushed an update to the lean toolchain because this PR was on a buggy version of the toolchain. WARNING: checking out old commits of this PR using v4.2.0-rc2 or v4.2.0-rc3 can cause lake clean to delete your mathlib folder! If you need to do so, make sure to delete lakefile.olean manually before running any lake commands.

@adomani
Copy link
Collaborator

adomani commented Oct 26, 2023

The code is above what I actually feel that I understand, but is this testing membership in the radical or whether some explicit power of the element lies in the ideal? More explicitly, could it not be the case that a larger power than what is being used would work?

As I said, I do not really understand the code, so I hope that this question is not just noise!

@robertylewis
Copy link
Member Author

It tests for membership in the radical.

To start, the goal is H₁ = 0, ..., Hₙ = 0 ⊢ P = 0. This follows if we can find Qᵢ and r such that Pʳ = Q₁H₁ + ... + QₙHₙ, i.e. P is in the radical of <Hᵢ>.

In Sage: introduce a new variable z and use a Grobner basis algorithm to compute hᵢ such that 1 = h₁H₁ + ... + hₙHₙ + h(1 - zP).

Take r to be the maximum power of z that appears in the hᵢs. Substitute 1/P (formally) for z and multiply 1 = h₁H₁ + ... + hₙHₙ + h(1 - zP) by to get our desired equation: the coeffs are Qᵢ = Pʳhᵢ[z ← 1/P].

So the Grobner basis computation produces r without a search. In principle, if there is an r that works, this should find it. Of course it may be computationally infeasible.

I'm not sure that this procedure makes any claim about finding a minimal r. This must depend on the details of the Grobner basis algorithm. In practice, on all the polyrith tests, it correctly finds r = 1 (and finds a minimal r > 1 on the new tests).

@adomani
Copy link
Collaborator

adomani commented Oct 26, 2023

Robert, thank you for the explanation! I had missed that there was a first Gröbner basis computation and then you used the degrees of the elements of that basis to get the bound on r: this clears my doubts!

In terms of improving feasibility, it may be good to have an option to ask Sage to check if the given element is a member of the ideal itself or whether it is contained in the radical: I expect these two computations to perform differently (where testing for membership in the ideal is likely often faster) and you may be satisfied with the answer to one of the two questions. I am thinking of something in the style of

  • polyrith -- check membership in the ideal;
  • polyrith! -- check membership in the radical.

Anyway, this is just a suggestion: I am very happy that polyrith even exists!

@robertylewis
Copy link
Member Author

robertylewis commented Oct 26, 2023

I thought about the two versions, but decided against it for a few reasons:

  • It's confusing for newcomers/people who don't want to think about how polyrith works, which is probably most people. They just want to know whether the goal is provable.
  • polyrith is self-replacing, so minor slowdowns don't matter nearly as much as for tactics that will remain source code.
  • It's not even clear to me that the radical problem is significantly slower on average. The computation has one more variable and similar degree. Grobner basis algorithms are fickle and adding an option to use a different term order would probably help a lot more than falling back to the original non-radical check.
  • Some non-scientific testing at the end of feat(tactic/polyrith): solve more problems by testing for membership in the radical mathlib3#15425 indicated that there's not much/any slowdown, although this isn't a strong data point!

@adomani
Copy link
Collaborator

adomani commented Oct 26, 2023

Rob, thanks for the further comments! I agree that specifying term orders normally has a much larger impact on feasibility of a Gröbner basis computation!

Thanks for taking the time to explain: it turns out that my comments were indeed noise, but I am grateful for the explanations, since they taught me something!

Comment on lines +26 to +33
[{", ".join(var_list)}] = P.gens()
p = P({goal_type})
gens = {eq_list} + [1 - p*aux]
I = P.ideal(gens)
coeffs = P(1).lift(I)
power = max(cf.degree(aux) for cf in coeffs)
coeffs = [P(cf.subs(aux = 1/p)*p^power) for cf in coeffs[:int(-1)]]
print(str(power)+';'+serialize_polynomials(coeffs))
Copy link
Member

@alexjbest alexjbest Nov 3, 2023

Choose a reason for hiding this comment

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

I don't really understand why this is done with this trick of checking if 1 is in the ideal <hyps, 1/goal>, I would expect singular/sage has a way to ask for a lift to the radical ideal directly which I would hope would be easier to maintain in the long run (especially if we have aspirations of making this work for non-char 0 rings properly). Did you consider this approach?
I'm not completely against merging this, but the whole thing is starting to look a bit scary ;)

Copy link
Member

Choose a reason for hiding this comment

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

After experimenting a bit I believe that using the builtin radical support, while probably a better algorithm underneath, is not very convenient to use with the way things are set up here, so I'm happy with this I guess

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I remember looking for this when I first implemented this (granted, a year and a half ago) and not turning up anything useful. Sage/Singular will check for membership in the radical just fine but extracting the power and coefficient is a pain. My impression is that what I do in this PR is a "standard" approach and that a native Sage implementation would do roughly the same, although I'm somewhat out of my element here.

Copy link
Member

Choose a reason for hiding this comment

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

When I read the sage docstring for radical earlier it said:

             From the Singular manual: A combination of the algorithms
            of Krick/Logar and Kemper is used. Works also in positive
            characteristic (Kempers algorithm).

but I have no idea how accurate that is!

Copy link
Member Author

Choose a reason for hiding this comment

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

Right -- but there's space between the membership check $p \in \sqrt{\langle H_1, \ldots, H_n \rangle}$, and explicitly producing $k$ and $Q_i$ such that $p^k = \sum Q_iH_i$. Sage's radical makes the former easy, but I don't see how to get from there to the latter. Maybe there's a way, I'd love to hear it.

(It sounds like you came to a similar conclusion, just leaving this comment for reference later!)

@alexjbest
Copy link
Member

maintainer merge

Copy link

github-actions bot commented Nov 3, 2023

🚀 Pull request has been placed on the maintainer queue by alexjbest.

@sgouezel sgouezel added this pull request to the merge queue Nov 7, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Nov 7, 2023
@robertylewis
Copy link
Member Author

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Nov 7, 2023
mathlib-bors bot pushed a commit that referenced this pull request Nov 7, 2023
)

In @hrmacbeth 's tutorial on `polyrith`, there were examples of problems that it could almost solve, but failed. The goal was not expressible as a linear combination of the hypotheses but a power of the goal was. 

```lean
example (x y z : ℚ) (h : x = y) (h2 : x * y = 0) : x + y*z = 0 :=
sorry
```

Mathematically, `x+y*z` is in the radical of the ideal generated by `x-y, x*y`. There's a "standard trick" for testing membership in the radical without a search for the proper exponent: see e.g. section 2.2 of [arxiv.org/pdf/1007.3615.pdf](https://arxiv.org/pdf/1007.3615.pdf) or 4.2 Prop 8 of Cox, Little, O'Shea.

This PR implements the trick in the Sage call made by `polyrith`. When the power returned is `n > 1`, we use `linear_combination (exp := n)` to check the certificate (#7789 ).

The `polyrith` test infrastructure still needs to be ported from mathlib3. All tests in the test file succeed when they are uncommented. A future PR will restore the old test suite.


Co-authored-by: Rob Lewis <[email protected]>
Co-authored-by: Mario Carneiro <[email protected]>
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Nov 7, 2023

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat: improve polyrith by testing for membership in the radical [Merged by Bors] - feat: improve polyrith by testing for membership in the radical Nov 7, 2023
@mathlib-bors mathlib-bors bot closed this Nov 7, 2023
@mathlib-bors mathlib-bors bot deleted the polyrith-radical branch November 7, 2023 18:02
grunweg pushed a commit that referenced this pull request Dec 15, 2023
)

In @hrmacbeth 's tutorial on `polyrith`, there were examples of problems that it could almost solve, but failed. The goal was not expressible as a linear combination of the hypotheses but a power of the goal was. 

```lean
example (x y z : ℚ) (h : x = y) (h2 : x * y = 0) : x + y*z = 0 :=
sorry
```

Mathematically, `x+y*z` is in the radical of the ideal generated by `x-y, x*y`. There's a "standard trick" for testing membership in the radical without a search for the proper exponent: see e.g. section 2.2 of [arxiv.org/pdf/1007.3615.pdf](https://arxiv.org/pdf/1007.3615.pdf) or 4.2 Prop 8 of Cox, Little, O'Shea.

This PR implements the trick in the Sage call made by `polyrith`. When the power returned is `n > 1`, we use `linear_combination (exp := n)` to check the certificate (#7789 ).

The `polyrith` test infrastructure still needs to be ported from mathlib3. All tests in the test file succeed when they are uncommented. A future PR will restore the old test suite.


Co-authored-by: Rob Lewis <[email protected]>
Co-authored-by: Mario Carneiro <[email protected]>
mathlib-bors bot pushed a commit that referenced this pull request May 24, 2024
The `polyrith` feature that checks for membership in the radical of the ideal fails if the target is 0. (That is, `polyrith` cannot prove `x - x = 0`.) This PR fixes this by checking (in Sage) whether the target is 0, and short circuiting if it is.

This example succeeded before #7790, fails after, and now succeeds again.

```lean
import Mathlib.Tactic.Polyrith

variable {R : Type*} [CommRing R]

example {x : R} (H : x = 1) : x = x := by polyrith 
```

This PR also renames a misleadingly named variable in the `polyrith` Python script.



Co-authored-by: Rob Lewis <[email protected]>
grunweg pushed a commit that referenced this pull request May 24, 2024
The `polyrith` feature that checks for membership in the radical of the ideal fails if the target is 0. (That is, `polyrith` cannot prove `x - x = 0`.) This PR fixes this by checking (in Sage) whether the target is 0, and short circuiting if it is.

This example succeeded before #7790, fails after, and now succeeds again.

```lean
import Mathlib.Tactic.Polyrith

variable {R : Type*} [CommRing R]

example {x : R} (H : x = 1) : x = x := by polyrith 
```

This PR also renames a misleadingly named variable in the `polyrith` Python script.



Co-authored-by: Rob Lewis <[email protected]>
callesonne pushed a commit that referenced this pull request Jun 4, 2024
The `polyrith` feature that checks for membership in the radical of the ideal fails if the target is 0. (That is, `polyrith` cannot prove `x - x = 0`.) This PR fixes this by checking (in Sage) whether the target is 0, and short circuiting if it is.

This example succeeded before #7790, fails after, and now succeeds again.

```lean
import Mathlib.Tactic.Polyrith

variable {R : Type*} [CommRing R]

example {x : R} (H : x = 1) : x = x := by polyrith 
```

This PR also renames a misleadingly named variable in the `polyrith` Python script.



Co-authored-by: Rob Lewis <[email protected]>
js2357 pushed a commit that referenced this pull request Jun 18, 2024
The `polyrith` feature that checks for membership in the radical of the ideal fails if the target is 0. (That is, `polyrith` cannot prove `x - x = 0`.) This PR fixes this by checking (in Sage) whether the target is 0, and short circuiting if it is.

This example succeeded before #7790, fails after, and now succeeds again.

```lean
import Mathlib.Tactic.Polyrith

variable {R : Type*} [CommRing R]

example {x : R} (H : x = 1) : x = x := by polyrith 
```

This PR also renames a misleadingly named variable in the `polyrith` Python script.



Co-authored-by: Rob Lewis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has been sent to bors. t-meta Tactics, attributes or user commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants