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(algebra/cubic_discriminant): remove custom cubic structure and replace by API about new to_poly #15088

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

hrmacbeth
Copy link
Member

@hrmacbeth hrmacbeth commented Jul 1, 2022

Currently, the leaf file algebra/cubic_discriminant by @Multramate introduces a custom cubic structure for polynomials of degree at most 3, which consists of a 4-tuple representing the coefficients plus some API.

As far as I can tell, this structure would be just as useful if it worked for N-tuples rather than just 4-tuples. So in this PR I introduce a constructor (temporarily named to_poly, following the existing name for cubics) for a polynomial over R given a vector fin N → R of the coefficients, and switch the cubic_discriminant file over to this, deleting the cubic structure.

If the idea is accepted as a good one, I will move the material about to_poly out of the cubic_discriminant file into data/polynomial somewhere. But for now, I am keeping everything in algebra/cubic_discriminant to demonstrate that it at least has the merit of decreasing the length of that file by 40 lines.

Zulip


Open in Gitpod

The PR generalizes some of the existing results from cubics to arbitrary-length polynomials, which depends on a variant of Vieta's formulas, coincidentally recently PR'd. The diff of this PR is therefore large, but all the changes I propose are (temporarily) in the cubic_discriminant file.

@hrmacbeth hrmacbeth added the RFC Request for comment label Jul 1, 2022
@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 1, 2022
Copy link
Collaborator

@vihdzp vihdzp left a comment

Choose a reason for hiding this comment

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

I like the idea, though I'm not very familiar with this API so someone else should give their opinion too.


lemma ne_zero_of_c_ne_zero (hc : P.c ≠ 0) : P.to_poly ≠ 0 :=
(or_imp_distrib.mp (or_imp_distrib.mp (or_imp_distrib.mp ne_zero).2).2).1 hc
lemma coeff_to_poly_ge_four (P : fin N → R) (n : ℕ) (hn : N ≤ n) : (to_poly P).coeff n = 0 :=
Copy link
Collaborator

Choose a reason for hiding this comment

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

These new names are nonsense for anything other than a cubic polynomial.


lemma degree_of_a_eq_zero (ha : P.a = 0) (hb : P.b ≠ 0) : P.to_poly.degree = 2 :=
by rw [of_a_eq_zero ha, degree_quadratic hb]
lemma degree_to_poly_le : (to_poly P).degree ≤ N :=
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't you prove this with degree_to_poly_lt and nat.lt_succ_iff?

@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 Aug 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request for comment too-late This PR was ready too late for inclusion in mathlib3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants