Skip to content

chore(Topology): replace continuity -> fun_prop#13994

Closed
grunweg wants to merge 10 commits intomasterfrom
MR-fun_prop-topology2
Closed

chore(Topology): replace continuity -> fun_prop#13994
grunweg wants to merge 10 commits intomasterfrom
MR-fun_prop-topology2

Conversation

@grunweg
Copy link
Copy Markdown
Contributor

@grunweg grunweg commented Jun 20, 2024

Addresses a few porting notes; I expect also a substantial speed-up of some proofs.
Change a few default values of proof fields from continuity to fun_prop; this required a few fix-ups downstream. (I checked a few instances; the failure seems related to Continuous.subtype_mk not always firing.)

This is almost (but not fully) exhaustive.


One proof gets partially un-automated (cont. -> partial fun_prop); if desired, I can revert this change.

Open in Gitpod

This is almost (but not fully exhaustive).
This changes the default values for Homeomorph... let's see if this breaks.

One proof gets partially un-automated (cont. -> partial fun_prop); if desired, I can revert this change.
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jun 20, 2024

PR summary 9aac757c82

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

No declarations were harmed in the making of this PR! 🐙

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.

· continuity
· continuity
· continuity
-- TODO: can the auto-param be converted to use `fun_prop` (instead of `continuity`)?
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The current auto-param uses continuity, which is slower... so, a good follow-up would be to make that use fun_prop as well. (This causes some breakage, which is why I'd like to not do so in this PR.)

def genLoopEquivOfUnique (N) [Unique N] : Ω^ N X x ≃ Ω X x where
toFun p :=
Path.mk ⟨fun t => p fun _ => t, by continuity⟩
Path.mk ⟨fun t => p fun _ => t, by
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My impression is that this version is significantly faster. That said, it would be great to reduce this to one line... perhaps mk_apply should be tagged with fun_prop?

@grunweg grunweg added awaiting-review t-topology Topological spaces, uniform spaces, metric spaces, filters labels Jun 20, 2024
@grunweg grunweg force-pushed the MR-fun_prop-topology2 branch from 6630106 to 3cdc4bf Compare June 20, 2024 14:16
@grunweg grunweg added the awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. label Jun 20, 2024
@grunweg grunweg force-pushed the MR-fun_prop-topology2 branch from 1be9017 to 693fb50 Compare June 20, 2024 14:59
@grunweg
Copy link
Copy Markdown
Contributor Author

grunweg commented Jun 20, 2024

CI is done (except for the linter), so let's benchmark this

@grunweg grunweg removed the awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. label Jun 20, 2024
@grunweg
Copy link
Copy Markdown
Contributor Author

grunweg commented Jun 20, 2024

!bench

@leanprover-bot
Copy link
Copy Markdown
Collaborator

Here are the benchmark results for commit 693fb50.
There were significant changes against commit 63b99ca:

  Benchmark                                               Metric         Change
  =============================================================================
+ ~Mathlib.Topology.ContinuousFunction.StoneWeierstrass   instructions   -12.6%
+ ~Mathlib.Topology.Homotopy.HomotopyGroup                instructions   -15.8%
+ ~Mathlib.Topology.MetricSpace.Perfect                   instructions   -72.0%

@ghost ghost added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jun 23, 2024
@MichaelStollBayreuth
Copy link
Copy Markdown
Contributor

General information:

build:                                                 -81.718 * 10⁹ (-0.0667%)

No file got slower by at least 10⁹ instructions.

No file got slower by at least 10%.

5 files got faster by at least 10⁹ instructions:

Mathlib.Topology.ContinuousFunction.StoneWeierstrass:  -31.429 * 10⁹ (-12.6%)
Mathlib.Topology.MetricSpace.Perfect:                  -21.279 * 10⁹ (-72.0%)
Mathlib.Topology.Homotopy.HomotopyGroup:               -15.318 * 10⁹ (-15.8%)
Mathlib.Topology.Homotopy.Path:                        -6.6815 * 10⁹ (-9.17%)
Mathlib.Analysis.NormedSpace.PiTensorProduct.InjectiveSeminorm:
                                                       -2.8943 * 10⁹ (-0.990%)

3 files got faster by at least 10%:

Mathlib.Topology.MetricSpace.Perfect:                                 -72.0%
Mathlib.Topology.Homotopy.HomotopyGroup:                              -15.8%
Mathlib.Topology.ContinuousFunction.StoneWeierstrass:                 -12.6%

@grunweg grunweg removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jun 23, 2024
@j-loreaux
Copy link
Copy Markdown
Contributor

j-loreaux commented Jun 24, 2024

I would like this PR to wait for #11092. I don't think we should be making fun_prop a default field until some of those bugs are fixed.

I have added this as a dependency to this PR, even though it isn't technically required in order to reflect this. Another maintainer can override me if they wish with an explanation.

@ghost ghost added the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Jun 24, 2024
@grunweg
Copy link
Copy Markdown
Contributor Author

grunweg commented Jun 25, 2024

If I find the time, I can also take out the default value changes (as the other ones are clearly good already). I'll see.

@ghost ghost added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jul 10, 2024
@ghost ghost removed the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Aug 12, 2024
@ghost
Copy link
Copy Markdown

ghost commented Aug 12, 2024

This PR/issue depends on:

@grunweg grunweg removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Aug 12, 2024
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Sep 6, 2024
@grunweg
Copy link
Copy Markdown
Contributor Author

grunweg commented Feb 15, 2026

Closing in favour of #35273.

@grunweg grunweg closed this Feb 15, 2026
@grunweg grunweg deleted the MR-fun_prop-topology2 branch February 15, 2026 10:51
mathlib-bors bot pushed a commit that referenced this pull request Mar 20, 2026
#35273)

Resurrected version of #13994. Let's look at the current fall-out and performance effect.
mathlib-bors bot pushed a commit that referenced this pull request Mar 20, 2026
#35273)

Resurrected version of #13994. Let's look at the current fall-out and performance effect.
mathlib-bors bot pushed a commit that referenced this pull request Mar 20, 2026
#35273)

Resurrected version of #13994. Let's look at the current fall-out and performance effect.
justus-springer pushed a commit to justus-springer/mathlib4 that referenced this pull request Mar 28, 2026
leanprover-community#35273)

Resurrected version of leanprover-community#13994. Let's look at the current fall-out and performance effect.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) t-topology Topological spaces, uniform spaces, metric spaces, filters

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants