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

chore: adaptations to nightly-2024-03-11 #11314

Merged
merged 60 commits into from Mar 13, 2024
Merged

Conversation

semorrison
Copy link
Contributor

@semorrison semorrison commented Mar 12, 2024

Open in Gitpod

Mathlib/Data/List/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/Num/Lemmas.lean Outdated Show resolved Hide resolved
semorrison and others added 6 commits March 12, 2024 18:55
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Collaborator

@grunweg grunweg left a comment

Choose a reason for hiding this comment

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

A few comments about "stray parts of the diff".

Mathlib/Analysis/BoundedVariation.lean Outdated Show resolved Hide resolved
Mathlib/Algebra/Homology/ExactSequence.lean Show resolved Hide resolved
Mathlib/Data/Nat/Digits.lean Outdated Show resolved Hide resolved
Mathlib/Topology/Gluing.lean Show resolved Hide resolved
grunweg added a commit that referenced this pull request Mar 12, 2024
mathlib-bors bot pushed a commit that referenced this pull request Mar 12, 2024
@mattrobball
Copy link
Collaborator

!bench

rw [← add_assoc]
-- Adaptation note: nightly-2024-03-11
-- I'm not sure why the `erw` is now needed here. It looks like it should work.
erw [hc]
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like it has a problem unfolding c itself so tries to unify Nat.rec and HSub.hsub

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you want to expand the comment?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like at diamond at this transparency between instHAdd and AddSemigroup.toAdd

Copy link
Collaborator

Choose a reason for hiding this comment

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

Expanded

| [], l' => l'
| l, [] => l
| a :: l, b :: l' => if a ≼ b then a :: merge l (b :: l') else b :: merge (a :: l) l'
| a :: l, b :: l' => if a ≼ b then a :: merge' l (b :: l') else b :: merge' (a :: l) l'
termination_by l₁ l₂ => length l₁ + length l₂
#align list.merge List.merge
Copy link
Contributor

Choose a reason for hiding this comment

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

Should List.merge being renamed be documented? (And should we mention leanprover-community/batteries#579 ?) I see there's an adaptation underway by @fgdorais

(And should the #align to List.merge' here? Or keep it with List.merge since that seems to be where the adaptation is going?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, oops, let's undo this in this PR. My mistake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I've revert this change, and instead incorporated the change from #11347

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 09b5587.Found no runs to compare against.

@kmill
Copy link
Contributor

kmill commented Mar 13, 2024

I've looked through all the changes, and everything (except for questions about List.merge) looks good to me.

@mattrobball
Copy link
Collaborator

!bench

Something terrible happened in injective resolutions

@semorrison
Copy link
Contributor Author

!bench

Something terrible happened in injective resolutions

It's still on overall improvement on instructions (very minor) and wall-clock, so we don't need to hold on this, right?

@mattrobball
Copy link
Collaborator

Don’t hold. It is probably something silly. Looking now

@mattrobball
Copy link
Collaborator

Added comment with label

@mattrobball
Copy link
Collaborator

🤦‍♂️

@semorrison semorrison merged commit 14fbc48 into bump/v4.8.0 Mar 13, 2024
16 checks passed
@mathlib-bors mathlib-bors bot deleted the bump/nightly-2024-03-11 branch March 13, 2024 03:16
dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
utensil pushed a commit that referenced this pull request Mar 26, 2024
Louddy pushed a commit that referenced this pull request Apr 15, 2024
uniwuni pushed a commit that referenced this pull request Apr 19, 2024
callesonne pushed a commit that referenced this pull request Apr 22, 2024
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.

None yet

9 participants