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] - chore: work around simp issues in future nightlies #11546

Closed
wants to merge 2 commits into from

Conversation

Ruben-VandeVelde
Copy link
Collaborator


Open in Gitpod

@Ruben-VandeVelde Ruben-VandeVelde added the awaiting-review The author would like community review of the PR label Mar 20, 2024
@@ -185,11 +185,13 @@ def toPreimages : J ⥤ Type v where
rw [← mem_preimage, preimage_preimage, mem_preimage]
convert h (g ≫ f); rw [F.map_comp]; rfl
map_id j := by
simp (config := { unfoldPartialApp := true }) only [MapsTo.restrict, Subtype.map, F.map_id]
-- Adaptation note: nightly-2024-03-16: simp [Subtype.map]
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're going to write an adaptation note, I think we ought to mention that unfoldPartialApp was removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

If the new simp only doesn't have any downsides (e.g. apparently here?), I think just delete the adaptation note.

If there is some downside, the adaptation note should contain enough information for someone coming along afterwards to reconstruct the original proof and see if it works again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The downside is that we need to add those _def lemmas at all, and remember when to use them rather than the name of the definition itself, so I've opted to clarify the comments. If we decide to stay with the new code, they're easy to find and remove.

@kmill
Copy link
Contributor

kmill commented Mar 21, 2024

Thanks!

bors r+

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Mar 21, 2024
@mathlib-bors
Copy link

mathlib-bors bot commented Mar 21, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: work around simp issues in future nightlies [Merged by Bors] - chore: work around simp issues in future nightlies Mar 21, 2024
@mathlib-bors mathlib-bors bot closed this Mar 21, 2024
@mathlib-bors mathlib-bors bot deleted the prepare-2024-03-16-simp branch March 21, 2024 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants