Skip to content

[ new ] Fin n as a refinement#2975

Open
gallais wants to merge 12 commits intoagda:masterfrom
gallais:nat-bounded
Open

[ new ] Fin n as a refinement#2975
gallais wants to merge 12 commits intoagda:masterfrom
gallais:nat-bounded

Conversation

@gallais
Copy link
Copy Markdown
Member

@gallais gallais commented Apr 8, 2026

This version has a better runtime representation
This version lets us have efficient implementations of things like splitAt and quotRem

I have been using this definition in my https://github.com/gallais/agda-tiling DSL and
it makes the library a lot faster.

I almost have feature parity with Data.Fin.Base (I have not implemented e.g. punchOut).

This version has a better runtime representation
This version lets us have efficient implementations of
things like splitAt and quotRem
@gallais gallais changed the title [ new ] Fin n as a refinement [ new ] Fin n as a refinement Apr 8, 2026
Comment thread src/Data/Nat/Bounded/Base.agda Outdated
Comment thread src/Data/Nat/Bounded/Base.agda
@jamesmckinna
Copy link
Copy Markdown
Collaborator

I think that this all looks great (but see also #2257 and #2292 which took a different angle of attack, focusing on modular arithmetic), but I'm a bit concerned about like-for-like emulation of the existing Data.Fin.Base API, when there are still many open issues drawing attention to... things which could be improved there. So I'm not sure I'd want to saddle us with the maintenance headache of fixing those issues on the new additions.

Also: where does this leave us wrt the 'propaganda' for DTFP with eg using Fin for type-safe/bounds-safe Data.Vec.Base.lookup?

Comment thread src/Data/Nat/Bounded/Base.agda Outdated
Comment thread src/Data/Nat/Bounded/Base.agda Outdated
gallais and others added 2 commits April 9, 2026 10:02
Co-authored-by: jamesmckinna <31931406+jamesmckinna@users.noreply.github.com>
Co-authored-by: jamesmckinna <31931406+jamesmckinna@users.noreply.github.com>
Comment thread src/Data/Nat/Bounded/Base.agda Outdated
Comment thread src/Data/Nat/Bounded/Base.agda Outdated
Comment thread src/Data/Nat/Bounded/Base.agda
Comment thread src/Data/Nat/Bounded/Base.agda Outdated
Comment thread src/Data/Nat/Bounded/Base.agda
Co-authored-by: jamesmckinna <31931406+jamesmckinna@users.noreply.github.com>
Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md
Comment thread CHANGELOG.md
gallais and others added 2 commits April 9, 2026 11:19
Co-authored-by: jamesmckinna <31931406+jamesmckinna@users.noreply.github.com>
Comment thread src/Data/Nat/Bounded/Base.agda Outdated
Copy link
Copy Markdown
Collaborator

@jamesmckinna jamesmckinna left a comment

Choose a reason for hiding this comment

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

This all looks fine; lots of potential downstream uses/improvements, but worth landing this sooner rather than later.

@jamesmckinna
Copy link
Copy Markdown
Collaborator

@gallais do you want this badged for v2.4?

@gallais
Copy link
Copy Markdown
Member Author

gallais commented Apr 13, 2026

I am in no rush but also happy to see it included. I have a deadline today but should be able to have
a look (and merge your proposals; glad the sniping worked!) later this week.

Comment thread src/Data/Bool/Properties.agda
@jamesmckinna
Copy link
Copy Markdown
Collaborator

Nice cleanup operation!

Copy link
Copy Markdown
Collaborator

@JacquesCarette JacquesCarette 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 nitpicks and one issue with naming. Otherwise, this looks great.

I believe this is also now the definition that 1lab uses.

Comment thread src/Data/Fin/Base.agda
Comment thread src/Data/Nat/Bounded/Base.agda
fsuc : ∀ {n} → Fin n → Fin (suc n)
fsuc = Refinement.map suc s<s

data View : ∀ {n} (k : Fin n) → Set where
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not keen on View as a name here. Yes, it is a view, but... I'm partial to AsX for a decent X as names for views - but definitely not dogmatic about it. AsFin obviously is a horrible name.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Used qualified, it gives us Fin.View and Fin.view which is not bad IMO?

Comment thread src/Data/Nat/Bounded/Base.agda
≤″⇒≤ : _≤″_ ⇒ _≤_
≤″⇒≤ (k , refl) = m≤m+n _ k

<″⇒< : _<″_ ⇒ _<_
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Might be wise to separate the "straight improvements" to stdlib from the addition of the new functionality in to a separate PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is used in this PR rather than being an orthogonal improvement which makes breaking it off
in a separate PR kind of annoying.

@jamesmckinna
Copy link
Copy Markdown
Collaborator

Maybe punchOut etc. could/should better follow the example of Data.List.Base.filter etc. and use Decidable _<_ as the engine of case analysis?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants