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

Wedges part 1 #1384

Merged
merged 6 commits into from
Nov 10, 2024
Merged

Wedges part 1 #1384

merged 6 commits into from
Nov 10, 2024

Conversation

zstone1
Copy link
Contributor

@zstone1 zstone1 commented Nov 6, 2024

A rather important construction for homotopies, this makes progress towards getting #1350 broken into smaller chunks

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

Reference: How to document

Reminder to reviewers

@affeldt-aist
Copy link
Member

Shouldn't wedgei be renamed to something more informative, like wedge_lift?

@affeldt-aist
Copy link
Member

In this commit , the Let lemma wedgei_p0 replaces the only use of wedge_pointE and can be used elsewhere as well. Maybe it is more useful and could be made a genuine Lemma (and potentially wedge_pointE removed?).

@zstone1
Copy link
Contributor Author

zstone1 commented Nov 9, 2024

I ended up with wedgei_p0 and wedge_pointE as separate lemmas for a performance hack for later PRs. I use wedge_pointE extensively when working with path concatenations (probably 100s of applications when fully unpacked). But you're right that it's redundant. The statement of the lemmas is unifiable. So let's just have wedge_p0 (now wedge_lift_p0E). If I do need other representations for performance reasons I should add them locally, so the hacks are self-contained.

Copy link
Member

@affeldt-aist affeldt-aist left a comment

Choose a reason for hiding this comment

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

In the last commit, I just fixed the changelog and I also took the liberty to rename wedge_lift_p0E to wedge_liftE because p0 is a local variable that I think should not be exposed. You may want to change that to a better name.

@zstone1 zstone1 merged commit f5097d5 into math-comp:master Nov 10, 2024
31 checks passed
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.

2 participants