Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

feat(data/{list/alist,finmap}): implicit key type #662

Merged
merged 4 commits into from
Feb 7, 2019

Conversation

spl
Copy link
Collaborator

@spl spl commented Feb 1, 2019

Make the key type α implicit in both alist and finmap. This brings these types into line with the underlying sigma and simplifies usage since α is inferred from the value function type β : α → Type v.

  • reviewed and applied the coding style: coding, naming
  • make sure definitions and lemmas are put in the right files
  • make sure definitions and lemmas are not redundant

Make the key type α implicit in both alist and finmap. This brings these
types into line with the underlying sigma and simplifies usage since α
is inferred from the value function type β : α → Type v.
@cipher1024 cipher1024 assigned cipher1024 and digama0 and unassigned cipher1024 Feb 1, 2019
src/data/list/alist.lean Outdated Show resolved Hide resolved
Copy link
Collaborator

@cipher1024 cipher1024 left a comment

Choose a reason for hiding this comment

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

This is similar to the array / darray module. Should we have a similar separation: finmap / dfinmap? I think that would be easier to use.

@digama0
Copy link
Member

digama0 commented Feb 2, 2019

If finmap here uses prod instead of sigma then probably not; it would duplicate a lot of theorems for no real gain. If it uses sigma then there isn't much point in having the special case at all. Maybe it could be a notation?

@cipher1024
Copy link
Collaborator

Why not make finmap a def? def finmap (α β : Type*) := dfinmap (λ x : α, β)

@cipher1024
Copy link
Collaborator

It has the benefit of being a functor and traversable.

Copy link
Collaborator

@cipher1024 cipher1024 left a comment

Choose a reason for hiding this comment

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

make a separate dfinmap and finmap definition so that we can define a functor and traversable instance on finmap

@spl
Copy link
Collaborator Author

spl commented Feb 4, 2019

This is similar to the array / darray module. Should we have a similar separation: finmap / dfinmap? I think that would be easier to use.

make a separate dfinmap and finmap definition so that we can define a functor and traversable instance on finmap

@cipher1024 I'm not sure I follow. I believe you're referring to the following?

structure d_array (n : nat) (α : fin n → Type u) :=
(data : Π i : fin n, α i)

def array (n : nat) (α : Type u) : Type u :=
d_array n (λ _, α)

Given that, are you suggesting a restructuring to have the following?

structure dfinmap {α : Type u} (β : α → Type v) : Type (max u v) :=
(entries : multiset (sigma β)) 
(nodupkeys : entries.nodupkeys)

def finmap (α : Type u) (β : Type v) : Type (max u v) :=
dfinmap (λ _ : α, β)

In other words, the existing finmap becomes the above dfinmap and finmap effectively becomes a prod-to-sigma-like wrapper.

First, I'm guessing you're in favor of the implicit {α : Type u} in dfinmap. I believe that, even in this layered approach, it's still useful.

Second, does this alleviate the duplication of definitions and theorems for dfinmap and finmap?

  • I think it would be useful to have a non-key-value-dependent finmap in practice. I predict that most uses of finmap will be non-dependent: all the uses I've found in Coq are non-dependent.
  • However, the dependent finmap is more general, and I would hate to duplicate it all for both. Would notation be better for the non-dependent finmap?

Finally, I believe the above restructuring is orthogonal to this PR, and I'd like to push this through first. There are also a lot more things I'd like to add to the existing finmap. Would it be acceptable to do those first before thinking about a non-dep finmap, or would it be better to introduce the non-dep finmap first?

@cipher1024
Copy link
Collaborator

Given that, are suggesting a restructuring to have the following?

structure dfinmap {α : Type u} (β : α → Type v) : Type (max u v) :=
(entries : multiset (sigma β))
(nodupkeys : entries.nodupkeys)

def finmap (α : Type u) (β : Type v) : Type (max u v) :=
dfinmap (λ _ : α, β)

Yes that's what I'm suggesting.

Finally, I believe the above restructuring is orthogonal to this PR, and I'd like to push this through first. There are also a lot more things I'd like to add to the existing finmap. Would it be acceptable to do those first before thinking about a non-dep finmap, or would it be better to introduce the non-dep finmap first?

Sure. After that, the functor instance should be simple enough. For the traversable instance, we should probably talk.

@johoelzl johoelzl merged commit 8a1de24 into master Feb 7, 2019
@johoelzl johoelzl deleted the alist-finmap-implicit-alpha branch February 7, 2019 12:13
@johoelzl
Copy link
Collaborator

johoelzl commented Feb 7, 2019

@cipher1024 and @digama0 given that both of you approved this, I merged the PR.

@cipher1024
Copy link
Collaborator

Thanks!

cipher1024 pushed a commit that referenced this pull request Feb 8, 2019
* feat(data/{list/alist,finmap}): implicit key type

Make the key type α implicit in both alist and finmap. This brings these
types into line with the underlying sigma and simplifies usage since α
is inferred from the value function type β : α → Type v.

* doc(data/list/alist): alist is stored as a linked list
@spl spl mentioned this pull request Feb 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants