Skip to content

[Merged by Bors] - chore: remove CoeFun instances where FunLike is available#17917

Closed
Vierkantor wants to merge 1 commit intomasterfrom
remove-coefun-test-3
Closed

[Merged by Bors] - chore: remove CoeFun instances where FunLike is available#17917
Vierkantor wants to merge 1 commit intomasterfrom
remove-coefun-test-3

Conversation

@Vierkantor
Copy link
Copy Markdown
Contributor

@Vierkantor Vierkantor commented Oct 18, 2024

During the port we found that FunLike is robust enough not to need an extra CoeFun shortcut. Let's see if that rule can be consistently applied to the whole of the library.

There is still duplication between FunLike and CoeFun for Grp, Mon, CommGrp and CommMon, which will need a more thorough fix. See also #17866.

This contains everything except the interval [1/4, 3/8] of the changes, and doesn't affect the benchmark.


Open in Gitpod

@Vierkantor Vierkantor added tech debt Tracking cross-cutting technical debt, see e.g. the "Technical debt counters" stream on zulip awaiting-bench This PR needs to be benchmarked before merging labels Oct 18, 2024
@Vierkantor Vierkantor changed the title Remove coefun test 3 chore: remove CoeFun instances where FunLike is available Oct 18, 2024
@Vierkantor
Copy link
Copy Markdown
Contributor Author

!bench

@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 18, 2024

PR summary 966a66bda2

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

- coeFun
- instance : CoeFun (P₁ ≃ᵃL[k] P₂) fun _ ↦ P₁ → P₂

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for script/declarations_diff.sh contains some details about this script.

@mathlib4-dependent-issues-bot mathlib4-dependent-issues-bot added the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Oct 18, 2024
@leanprover-bot
Copy link
Copy Markdown
Collaborator

Here are the benchmark results for commit 469ff48.
There were significant changes against commit 672f75b:

  Benchmark                                     Metric         Change
  ===================================================================
- ~Mathlib.Analysis.InnerProductSpace.Adjoint   instructions     8.5%

@Vierkantor
Copy link
Copy Markdown
Contributor Author

Nearly there!

@Vierkantor Vierkantor force-pushed the remove-coefun-test-3 branch from 469ff48 to b0a7bee Compare October 18, 2024 15:05
@Vierkantor
Copy link
Copy Markdown
Contributor Author

!bench

@leanprover-bot
Copy link
Copy Markdown
Collaborator

Here are the benchmark results for commit b0a7bee.
There were no significant changes against commit 672f75b.

@Vierkantor Vierkantor removed the awaiting-bench This PR needs to be benchmarked before merging label Oct 18, 2024
@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented Oct 19, 2024

bors d+

@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Oct 19, 2024

✌️ Vierkantor can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@github-actions github-actions bot added the delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). label Oct 19, 2024
@mathlib4-dependent-issues-bot mathlib4-dependent-issues-bot removed the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Oct 19, 2024
@mathlib4-dependent-issues-bot
Copy link
Copy Markdown
Collaborator

This PR/issue depends on:

@Vierkantor Vierkantor force-pushed the remove-coefun-test-3 branch from b0a7bee to 966a66b Compare October 21, 2024 08:46
@Vierkantor
Copy link
Copy Markdown
Contributor Author

!bench

@Vierkantor
Copy link
Copy Markdown
Contributor Author

Just to make sure the situation didn't change.

@leanprover-bot
Copy link
Copy Markdown
Collaborator

Here are the benchmark results for commit 966a66b.Found no runs to compare against.

@Vierkantor
Copy link
Copy Markdown
Contributor Author

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Oct 21, 2024
mathlib-bors bot pushed a commit that referenced this pull request Oct 21, 2024
During the port we found that `FunLike` is robust enough not to need an extra `CoeFun` shortcut. Let's see if that rule can be consistently applied to the whole of the library.

There is still duplication between `FunLike` and `CoeFun` for `Grp`, `Mon`, `CommGrp` and `CommMon`, which will need a more thorough fix. See also #17866.

This contains everything except the interval [1/4, 3/8] of the changes, and doesn't affect the benchmark.
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Oct 21, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: remove CoeFun instances where FunLike is available [Merged by Bors] - chore: remove CoeFun instances where FunLike is available Oct 21, 2024
@mathlib-bors mathlib-bors bot closed this Oct 21, 2024
@mathlib-bors mathlib-bors bot deleted the remove-coefun-test-3 branch October 21, 2024 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). ready-to-merge This PR has been sent to bors. tech debt Tracking cross-cutting technical debt, see e.g. the "Technical debt counters" stream on zulip

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants