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

Pi irrational #1351

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

Conversation

affeldt-aist
Copy link
Member

Motivation for this change

@LaurenceRideau

Checklist
  • added corresponding entries in CHANGELOG_UNRELEASED.md
  • added corresponding documentation in the headers

Reference: How to document

Reminder to reviewers

@affeldt-aist affeldt-aist added the enhancement ✨ This issue/PR is about adding new features enhancing the library label Oct 14, 2024
@affeldt-aist affeldt-aist added this to the 1.6.0 milestone Oct 14, 2024
@affeldt-aist affeldt-aist modified the milestones: 1.6.0, 1.7.0 Oct 23, 2024
@affeldt-aist affeldt-aist marked this pull request as ready for review October 30, 2024 15:07
Copy link
Contributor

@zstone1 zstone1 left a comment

Choose a reason for hiding this comment

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

Overall a nice milestone that we can do this in ~600 lines now. Sadly at least 50% of that seems to be about measurable/differentiable/integrable stuff that ought to be automated. But it's a nice application nonetheless.

Moving some of the files around as you mentioned in your comments is probably good. Especially the polynomial stuff will be nice to have independently. And the filename thing should be an easy fix. Otherwise I don't see any blockers

End derive_horner.

Local Open Scope classical_set_scope.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we don't have just continuous (horner p))? Seems like that would imply all of this, but I am quite unfamiliar with mathcomps polynomials so I expect i am missing something.

Local Notation "f ^` ()" := (derive1 f) : classical_set_scope.

(* connect MathComp's polynomials with analysis *)
Section derive_horner.
Copy link
Contributor

Choose a reason for hiding this comment

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

All this stuff about polynomials and derivates is great. It should definitely live in a more accesible place. We'll want it for the full strength Taylor's theorem, for example

by rewrite -mulrDl hornerD.
Qed.

Definition intfsin n := \int[mu]_(x in `[0, pi]) (fsin n x).
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose most of these should be Local Definition, and Local Lemma if anything. No reason for these to leak outside the file if it can be helped.

Let abx x : 0 < x < pirat -> 0 < a na - b nb * x.
Proof.
move=> /andP[x0]; rewrite subr_gt0.
rewrite /pirat /exercise6.pirat ltr_pdivlMr 1?mulrC//.
Copy link
Contributor

Choose a reason for hiding this comment

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

The explicit dependency on the filename is a little jarring. I would prefer at most one of the following to be true, ideally neither

  • The file is called exercise6 without any context
  • The text of the filename explicitly appears in the proof script.

@affeldt-aist affeldt-aist modified the milestones: 1.7.0, 1.8.0 Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ This issue/PR is about adding new features enhancing the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants