Skip to content

chore: define Complex.UnitDisc using Subsemigroup.unitBall#39385

Open
ocfnash wants to merge 1 commit into
leanprover-community:masterfrom
ocfnash:ocfnash/unitdisc
Open

chore: define Complex.UnitDisc using Subsemigroup.unitBall#39385
ocfnash wants to merge 1 commit into
leanprover-community:masterfrom
ocfnash:ocfnash/unitdisc

Conversation

@ocfnash
Copy link
Copy Markdown
Contributor

@ocfnash ocfnash commented May 14, 2026


Open in Gitpod

@ocfnash ocfnash added the easy < 20s of review time. See the lifecycle page for guidelines. label May 14, 2026
@github-actions
Copy link
Copy Markdown

PR summary dcd756f437

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

+ Subsemigroup.mem_unitBall
- instCommSemigroup

You can run this locally as follows
## from your `mathlib4` directory:
git clone https://github.com/leanprover-community/mathlib-ci.git ../mathlib-ci

## summary with just the declaration names:
../mathlib-ci/scripts/pr_summary/declarations_diff.sh <optional_commit>

## more verbose report:
../mathlib-ci/scripts/pr_summary/declarations_diff.sh long <optional_commit>

The doc-module for scripts/pr_summary/declarations_diff.sh in the mathlib-ci repository contains some details about this script.


No changes to strong technical debt.
No changes to weak technical debt.

@github-actions github-actions Bot added the t-analysis Analysis (normed *, calculus) label May 14, 2026
Copy link
Copy Markdown
Member

@ADedecker ADedecker left a comment

Choose a reason for hiding this comment

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

Thanks!

bors merge

@mathlib-triage mathlib-triage Bot added the ready-to-merge This PR has been sent to bors. label May 14, 2026
@eric-wieser
Copy link
Copy Markdown
Member

eric-wieser commented May 14, 2026

bors r-

Can you elaborate in the PR description on the benefit of this? Inferring all the instances as inferInstanceAs <| SemigroupWithZero (ball _ _) except this one which is derived from Subsemigroup.unitBall feels like it will just slow down instance unification.

@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors Bot commented May 14, 2026

Canceled.

Address comments or fix if necessary, and then someone with permission can run bors r+.

@mathlib-triage mathlib-triage Bot removed the ready-to-merge This PR has been sent to bors. label May 14, 2026
@ocfnash
Copy link
Copy Markdown
Contributor Author

ocfnash commented May 14, 2026

The motivation is to have some local consistency between the definitions of Circle and Complex.UnitDisc. At the moment, there is a mild style inconsistency since Circle uses Submonoid.unitSphere whereas Complex.UnitDisc uses a raw Metric.ball.

The background is that we are about to add Complex.closedUnitDisc and so the question arose about which pattern to follow. I decided that we should follow Circle and bring Complex.UnitDisc in line.

I admit I did not consider the potential performance point you raise but I would be very surprised if it mattered.

/-- Coercion to `ℂ`. -/
@[coe] protected def coe : 𝔻 → ℂ := Subtype.val

instance instCommSemigroup : CommSemigroup UnitDisc := inferInstanceAs <| CommSemigroup (ball _ _)
Copy link
Copy Markdown
Member

@eric-wieser eric-wieser May 15, 2026

Choose a reason for hiding this comment

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

Note that Circle does not use deriving, presumably for the performance reasons I mention:

def Circle : Type := Submonoid.unitSphere ℂ
deriving TopologicalSpace

instance instCommGroup : CommGroup Circle := inferInstanceAs <| CommGroup (sphere _ _)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

easy < 20s of review time. See the lifecycle page for guidelines. t-analysis Analysis (normed *, calculus)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants