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(Tactic/ComputeDegree): add helper lemmas for compute_degree_le #5978

Closed
wants to merge 5 commits into from

Conversation

adomani
Copy link
Collaborator

@adomani adomani commented Jul 18, 2023

This PR is a prequel to #5882: it simply adds the helper lemmas about polynomials that the tactic uses.

Zulip discussion


Open in Gitpod

@adomani adomani added the awaiting-review The author would like community review of the PR label Jul 18, 2023
@adomani adomani mentioned this pull request Jul 18, 2023
1 task
section semiring
variable [Semiring R]

theorem add {a b : Nat} {f g : R[X]} (hf : natDegree f ≤ a) (hg : natDegree g ≤ b) :
Copy link
Member

Choose a reason for hiding this comment

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

IMHO, this can go near Polynomial.natDegree_add_le, e.g., under name Polynomial.natDegree_add_le_of_le. See norm_add_le_of_le for an example of this pattern already in the library.

Same for most of the lemmas below.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yuri, thank you for the review!

I moved almost all the lemmas to Data/Pol/Deg/Defs. In the new file, I left only the lemmas of the form natDegree ↑n ≤ 0. Given that the stronger form natDegree ↑n = 0 is already present, adding these to the "main" library seems unwanted clutter.

Let me know if you want me to move these lemmas also out of the file, and I can do it!

natDegree_pow_le.trans (Nat.mul_le_mul rfl.le ‹_›)

theorem C_le (a : R) : natDegree (C a) ≤ 0 := (natDegree_C a).le
theorem nat_cast_le (n : Nat) : natDegree (n : R[X]) ≤ 0 := (natDegree_nat_cast _).le
Copy link
Member

Choose a reason for hiding this comment

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

BTW, do you handle (2 : R[X]) (a.k.a. OfNat.ofNat 2 : R[X]) in your tactic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think so. It seems that special support in this case only touches 0, 1:

example [Semiring R] : natDegree (0 : R[X]) ≤ 0 := by
  compute_degree_le

example [Semiring R] : degree (1 : R[X]) ≤ 0 := by
  compute_degree_le

example [Semiring R] : natDegree (2 : R[X]) ≤ 0 := by
  compute_degree_le

example [Semiring R] : degree (OfNat.ofNat 2 : R[X]) ≤ 0 := by
  compute_degree_le

all work with the current tactic.

However, your comment highlighted a bug:

example [Semiring R] : degree (OfNat.ofNat 0 : R[X]) ≤ 0 := by
  compute_degree_le

example [Semiring R] : degree (OfNat.ofNat 1 : R[X]) ≤ 0 := by
  compute_degree_le
/-
tactic 'apply' failed, failed to unify
  degree ↑?n ≤ 0
with
  degree (OfNat.ofNat 1) ≤ ?b
-/

both fail. I'll investigate this in the other PR.

Copy link
Member

Choose a reason for hiding this comment

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

These OfNat instances use Zero and One, not Nat.cast.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think that I fixed this in the main tactic by using Expr.numeral? and being more systematic.

I find the new coercion/numeral system somewhat confusing. Also, I still think that the special support affects 0, 1 in this case, but from 2 onward it appears to be uniform.

@@ -285,6 +285,8 @@ theorem natDegree_nat_cast (n : ℕ) : natDegree (n : R[X]) = 0 := by
simp only [← C_eq_nat_cast, natDegree_C]
#align polynomial.nat_degree_nat_cast Polynomial.natDegree_nat_cast

theorem degree_nat_cast_le (n : Nat) : degree (n : R[X]) ≤ 0 := degree_le_of_natDegree_le (by simp)
Copy link
Member

Choose a reason for hiding this comment

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

Here and below (same for Int):

Suggested change
theorem degree_nat_cast_le (n : Nat) : degree (n : R[X]) ≤ 0 := degree_le_of_natDegree_le (by simp)
theorem degree_nat_cast_le (n : ) : degree (n : R[X]) ≤ 0 := degree_le_of_natDegree_le (by simp)

@urkud
Copy link
Member

urkud commented Jul 18, 2023

Thanks! I pushed a minor golf. Feel free to either accept it or revert it.
bors d+

@bors
Copy link

bors bot commented Jul 18, 2023

✌️ adomani can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@github-actions github-actions bot added delegated and removed awaiting-review The author would like community review of the PR labels Jul 18, 2023
@urkud urkud added the t-algebra Algebra (groups, rings, fields etc) label Jul 18, 2023
@adomani
Copy link
Collaborator Author

adomani commented Jul 19, 2023

bors r+

bors bot pushed a commit that referenced this pull request Jul 19, 2023
…#5978)

This PR is a prequel to #5882: it simply adds the helper lemmas about polynomials that the tactic uses.

[Zulip discussion ](https://leanprover.zulipchat.com/#narrow/stream/144837-PR-reviews/topic/.235882.20.60compute_degree_le.60)



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
@bors
Copy link

bors bot commented Jul 19, 2023

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title feat(Tactic/ComputeDegree): add helper lemmas for compute_degree_le [Merged by Bors] - feat(Tactic/ComputeDegree): add helper lemmas for compute_degree_le Jul 19, 2023
@bors bors bot closed this Jul 19, 2023
@bors bors bot deleted the adomani_trivial_cdle_lemmas branch July 19, 2023 02:48
semorrison pushed a commit that referenced this pull request Aug 14, 2023
…#5978)

This PR is a prequel to #5882: it simply adds the helper lemmas about polynomials that the tactic uses.

[Zulip discussion ](https://leanprover.zulipchat.com/#narrow/stream/144837-PR-reviews/topic/.235882.20.60compute_degree_le.60)



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated t-algebra Algebra (groups, rings, fields etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants