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

Adapt to hornerE respecting exponents - math-comp's PR #923 #81

Merged
merged 4 commits into from
Sep 20, 2022

Conversation

SnarkBoojum
Copy link

This one-patch is making Abel compile with both the current hornerE and the proposed revision.

@SnarkBoojum
Copy link
Author

I'm pretty sure the second patch works with the current hornerE, but I'm not sure it still flies with the proposed new one ; I'm going to check.

@SnarkBoojum
Copy link
Author

Ok, now I think we have something!

Copy link
Contributor

@proux01 proux01 left a comment

Choose a reason for hiding this comment

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

Nitpicking

@@ -416,7 +416,7 @@ move=> n_gt0; have [->|xN0] := eqVneq x 0.
rewrite [LHS](@all_roots_prod_XsubC _ _ ws).
- by rewrite (monicP _) ?monic_XnsubC// scale1r big_map big_enum.
- by rewrite size_XnsubC// size_map size_enum_ord.
- rewrite all_map; apply/allP => i _ /=; rewrite /root !hornerE hornerXn.
- rewrite all_map; apply/allP => i _ /=; rewrite /root !hornerE ?hornerXn.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- rewrite all_map; apply/allP => i _ /=; rewrite /root !hornerE ?hornerXn.
- rewrite all_map; apply/allP => i _ /=; rewrite /root !hornerE ?hornerXn.
(* FIXME: remove ?hornerXn above once requiring MathComp >= 1.16.0 *)

@@ -433,7 +433,7 @@ Lemma dvdp_minpoly_Xn_subn E :
(x ^+ p)%R \in E -> minPoly E x %| ('X^p - (x ^+ p)%:P).
Proof using.
move=> xpE; have [->|p_gt0] := posnP p; first by rewrite !expr0 subrr dvdp0.
by rewrite minPoly_dvdp /root ?poly_XnsubC_over// !hornerE hornerXn subrr.
by rewrite minPoly_dvdp /root ?poly_XnsubC_over// !hornerE ?hornerXn subrr.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
by rewrite minPoly_dvdp /root ?poly_XnsubC_over// !hornerE ?hornerXn subrr.
by rewrite minPoly_dvdp /root ?poly_XnsubC_over// !hornerE ?hornerXn subrr.
(* FIXME: remove ?hornerXn above once requiring MathComp >= 1.16.0 *)

@@ -1505,7 +1505,7 @@ have Cchar := Cchar => p_neq0; split.
move=> /radicalP[]; case: i => // i in epw * => _ uik.
pose v := i.+1.-root (iota (u ^+ i.+1)).
have : ('X ^+ i.+1 - (v ^+ i.+1)%:P).[iota u] == 0.
by rewrite !hornerE hornerXn rootCK// rmorphX subrr.
by rewrite !hornerE ?hornerXn rootCK// rmorphX subrr.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
by rewrite !hornerE ?hornerXn rootCK// rmorphX subrr.
by rewrite !hornerE ?hornerXn rootCK// rmorphX subrr.
(* FIXME: remove ?hornerXn above once requiring MathComp >= 1.16.0 *)

@@ -432,7 +432,7 @@ have Xxn0 : ('X - y%:P) ^+ n != 0 by rewrite ?expf_neq0 ?polyXsubC_eq0.
apply/eqP; rewrite eqn_leq mup_leq ?mup_geq//.
have [->|Nxy] := eqVneq x y.
by rewrite /= dvdpp ?dvdp_Pexp2l ?size_XsubC ?ltnn.
by rewrite dvd1p dvdp_XsubCl /root horner_exp !hornerE expf_neq0// subr_eq0.
by rewrite dvd1p dvdp_XsubCl /root !hornerE ?horner_exp ?hornerE expf_neq0// subr_eq0.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
by rewrite dvd1p dvdp_XsubCl /root !hornerE ?horner_exp ?hornerE expf_neq0// subr_eq0.
by rewrite dvd1p dvdp_XsubCl /root !hornerE ?horner_exp ?hornerE expf_neq0// subr_eq0.
(* FIXME: remove ?horner_exp ?hornerE above once requiring MathComp >= 1.16.0 *)

@SnarkBoojum
Copy link
Author

I followed the suggestion.

@proux01
Copy link
Contributor

proux01 commented Sep 20, 2022

@CohenCyril CI green, this is backward compatible, could you merge? (if you agree with the upstream PR math-comp/math-comp#923 of course)

@CohenCyril CohenCyril merged commit 56cbd91 into math-comp:master Sep 20, 2022
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