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

feat(tactic/polyrith): solve more problems by testing for membership in the radical #15425

Closed
wants to merge 177 commits into from

Conversation

robertylewis
Copy link
Member

@robertylewis robertylewis commented Jul 16, 2022

In @hrmacbeth 's tutorial, there were examples of problems that polyrith 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. Sage can compute the radical and test for membership. Once it's confirmed that some power of the goal is in the ideal, we can count up until we find which power it is. The certificate returned is an application of pow_eq_zero with this power, then a call to linear_combination like normal.

example (x y z : ℚ) (h : x = y) (h2 : x * y = 0) : x + y*z = 0 :=
begin
  refine pow_eq_zero' 2 _; -- goal: (x + y*z)^2 = 0
  linear_combination ((-1) * y * z ^ 2 + 1 * x) * h + (1 * z ^ 2 + 2 * z + 1) * h2
end

There's an open question here: instead of a boolean test for membership in the radical, then naively testing every power for membership in the original ideal, is there a way to extract the power more efficiently during the computation of the radical? This is both a mathematical question and a Sage/Singular API question.

But in any case, the naive approach works on small enough problems (the degree 6 example from Heather's tutorial spends 400ms calling Sage including the communcation overhead), and should have no effect on performance in the cases polyrith already solves.


Tests do not build yet. #15292 also affects the tests output. I'll update this one when that one is merged, or stack them if/when I have the energy.

Open in Gitpod

@robertylewis robertylewis added awaiting-review The author would like community review of the PR t-meta Tactics, attributes or user commands labels Jul 16, 2022
@erdOne
Copy link
Member

erdOne commented Jul 16, 2022

I might not know what I am talking about, but it might be possible to perform some kind of binary search on the exponent in the range of say 1~64 if you have 7 queries available and testing for large polynomials isn't that much more costly than small ones?

@robertylewis
Copy link
Member Author

I might not know what I am talking about, but it might be possible to perform some kind of binary search on the exponent in the range of say 1~64 if you have 7 queries available and testing for large polynomials isn't that much more costly than small ones?

Interesting thought! This assumes that 64 (or whatever we choose) is high enough for most of the problems we see and that the exponents are approximately uniformly distributed in that range. I'd guess (without any evidence) that in typical problems, very low exponents will be more common. But this is really just a gut feeling and I have no test suite of typical problems to check. I'd also have to think about how the ideal membership test scales with the exponent. There is a cost there but I don't know how severe.

@tb65536
Copy link
Collaborator

tb65536 commented Jul 16, 2022

If you're going to binary search, would it make more sense to just repeatedly double the exponent?

@mathlib-dependent-issues-bot mathlib-dependent-issues-bot added the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Jul 16, 2022
@robertylewis
Copy link
Member Author

It turns out there's a "standard trick" for doing this without search: see e.g. section 2.2 of https://arxiv.org/pdf/1007.3615.pdf . I'll investigate whether Sage/Singular supports this trick out of the box or if we need to implement it ourselves. Marking this PR as WIP for now.

@robertylewis robertylewis added WIP Work in progress and removed awaiting-review The author would like community review of the PR labels Jul 16, 2022
@tb65536
Copy link
Collaborator

tb65536 commented Jul 16, 2022

It turns out there's a "standard trick" for doing this without search: see e.g. section 2.2 of https://arxiv.org/pdf/1007.3615.pdf . I'll investigate whether Sage/Singular supports this trick out of the box or if we need to implement it ourselves. Marking this PR as WIP for now.

Ooh, that's clever!

@robertylewis
Copy link
Member Author

Note to self, the json changes here will need to be adapted when #15429 lands.

robertylewis and others added 13 commits July 22, 2022 12:29
… a boolean algebra (#15606)

Abstract the construction of `boolean_algebra (finset α)`.
This change means that `option (rbmap string α)` can now serialize
* Use `namespace isometry`.
* Add lemmas like `isometry.preimage_ball`.
…nstances (#15602)

We move the trivial `pi.has_sdiff`, `pi.has_compl`, and `Prop.has_compl` instances to `order/basic.lean`. The main effect of this is that `rᶜ` for the complement of a relation is now available basically anywhere.
…` as `p.support.max` (#15199)

This PR redefines `polynomial.degree p`:
* old: `p.support.sup coe`
* new: `p.support.max`.

The two definitions are defeq and relatively few changes are required.

Weirdness: `open finset` seems to no longer work consistently.  This is the largest source of differences.

In particular, the file `ring_theory/polynomial/cyclotomic/basic` only changed because I added `finset.` in several places.
Define the upper/lower set generated by a set.

Co-authored-by: Peter Nelson
Since we're interfacing with ordinals and since `pgame` holds no data, we simplify `nim` and allow it to be noncomputable. We need to give `nim` the `noncomputable!` attribute to avoid a VM compilation bug.
…ween edge densities (#15353)

Auxiliary lemma for Szemerédi Regularity Lemma.

Co-authored-by: Bhavik Mehta <bhavik@mehta8@gmail.com>
…ion.atomise` (#15350)

Auxiliary lemmas for Szemerédi Regularity Lemma.

Co-authored-by: Bhavik Mehta <bhavik.mehta8@gmail.com>
@robertylewis
Copy link
Member Author

I've implemented the "standard trick" (see also 4.2 Prop 8 of Cox, Little, O'Shea). This is still pending docs, test updates, and #15428 (which I think is ready to go).

While this approach is clearly better overall than the old "search up" approach, there's one downside, namely, it doesn't fall back to the old behavior in the common case where the exponent is 1. Instead we solve a more general problem and deduce from that that n = 1. I don't know how bad of a performance hit this is -- grobner basis computations are hard to predict. The alternative is to keep polyrith using the old behavior and add polyrith! for the new fancy version.

@robertylewis robertylewis added the awaiting-review The author would like community review of the PR label Aug 2, 2022
@eric-wieser
Copy link
Member

It looks like you've accidentally rebased master on top of your branch, as there are 170 commits now!

@robertylewis
Copy link
Member Author

Yes, there were merge conflicts with #15429 and I didn't resolve cleanly, apparently. This is effectively a reset of the PR -- the new implementation is unrelated to the old. I'll squash and force push before cleaning up the rest.

cd8620d is the relevant commit.

@robertylewis
Copy link
Member Author

And I've marked this awaiting-review mostly for comments on:

While this approach is clearly better overall than the old "search up" approach, there's one downside, namely, it doesn't fall back to the old behavior in the common case where the exponent is 1. Instead we solve a more general problem and deduce from that that n = 1. I don't know how bad of a performance hit this is -- grobner basis computations are hard to predict. The alternative is to keep polyrith using the old behavior and add polyrith! for the new fancy version.

@miguelmarco
Copy link

There's an open question here: instead of a boolean test for membership in the radical, then naively testing every power for membership in the original ideal, is there a way to extract the power more efficiently during the computation of the radical? This is both a mathematical question and a Sage/Singular API question.

One possible way is to use the saturation method to get the needed exponent.

@miguelmarco
Copy link

Also, you could include a proof of this result:

example {F : Type} [field F] (c : F) (f : polynomial F) (rel : f * (c • X -1) = 1)  : c = 0 

and ask sage for a lift to the ideal generated by the hypothesis plus c*X - 1 (being X an extra variable). Then you can use sage's answer to create a proof of the needed hypothesis, and apply it.

@miguelmarco
Copy link

While this approach is clearly better overall than the old "search up" approach, there's one downside, namely, it doesn't fall back to the old behavior in the common case where the exponent is 1. Instead we solve a more general problem and deduce from that that n = 1. I don't know how bad of a performance hit this is -- grobner basis computations are hard to predict. The alternative is to keep polyrith using the old behavior and add polyrith! for the new fancy version.

I have done some simple benchmarking with five variables, and it doesn't seem to be a problem. When the previous method was fast (say, in the order of few seconds or less), the new method was also fast. In fact, in many instances it even happens that the new method is (way) faster!

So it is perfectly possible that the new behaviour would be preferable for the speed boost alone.

bors bot pushed a commit that referenced this pull request Mar 24, 2023
…onent := n }` (#15428)

#15425 will solve problems where the goal is not a linear combination of hypotheses, but a *power* of the goal is. This PR provides a more natural certificate for these proofs. Writing `linear_combination ... with { exponent  := 2 }` will square the goal before subtracting the linear combination. In principle this could be useful even outside of `polyrith`. 



Co-authored-by: Rob Lewis <rob.y.lewis@gmail.com>
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
@mathlib-dependent-issues-bot mathlib-dependent-issues-bot removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Mar 24, 2023
@mathlib-dependent-issues-bot
Copy link
Collaborator

@eric-wieser eric-wieser added the not-too-late This PR was ready at the point mathlib3 was frozen: we will try to merge it and port it to mathlib4 label Jul 15, 2023
@eric-wieser eric-wieser requested a review from a team as a code owner October 16, 2023 15:12
bors bot pushed a commit to leanprover-community/mathlib4 that referenced this pull request Oct 20, 2023
[mathlib3#15428](leanprover-community/mathlib#15428) added an exponent option to `linear_combination`. The idea is that `linear_combination (exp := 2) ...` will take a linear combination of hypotheses adding up to the *square* of the goal. This is only mildly useful on its own, but it's a very useful certificate syntax for [mathlib3#15425](leanprover-community/mathlib#15425).



Co-authored-by: Rob Lewis <rob.y.lewis@gmail.com>
@eric-wieser
Copy link
Member

Closing, since this was implemented in leanprover-community/mathlib4#7790

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review The author would like community review of the PR not-too-late This PR was ready at the point mathlib3 was frozen: we will try to merge it and port it to mathlib4 t-meta Tactics, attributes or user commands WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet