Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New property merge-is-superlist #1950

Merged
merged 8 commits into from
Jun 20, 2023

Conversation

andreasabel
Copy link
Member

Proofs that both lists involved in a merge are sublists of the result of the merge.

@Taneb
Copy link
Member

Taneb commented Apr 24, 2023

I think these functions are incorrectly named; merge is the superlist, not the sublist. But on the other hand I don't think we ever use the term superlist, so I'm not sure what to call them

@andreasabel
Copy link
Member Author

@Taneb wrote:

I think these functions are incorrectly named; merge is the superlist, not the sublist. But on the other hand I don't think we ever use the term superlist, so I'm not sure what to call them

Thanks for the comment, now I am using superlist.

@andreasabel andreasabel changed the title New property merge-is-sublist New property merge-is-superlist Apr 25, 2023
@jamesmckinna
Copy link
Contributor

Nice! Thanks for the response to reviews... I fully sympathise with the difficulty of finding names, here where they are imported from deep below the hierarchy (and Heterogeneous doesn't immediately suggest that its contents will be relevant to the homogeneous case)

@jamesmckinna
Copy link
Contributor

Regarding superlist: do we have a universal property of merge as a supremum of the two sublists?

@Taneb
Copy link
Member

Taneb commented Apr 25, 2023

Regarding superlist: do we have a universal property of merge as a supremum of the two sublists?

@jamesmckinna I couldn't find it. Could you make an issue for it, because I think it's important to have and also doesn't need to be done in this PR (I think it might need stronger preconditions on the lists)

@andreasabel
Copy link
Member Author

Regarding superlist: do we have a universal property of merge as a supremum of the two sublists?

@jamesmckinna I couldn't find it. Could you make an issue for it, because I think it's important to have and also doesn't need to be done in this PR (I think it might need stronger preconditions on the lists)

Yeah,merge is only a supremum for totally ordered lists.
On arbitrary lists, merge is only a supremum up to permutation and I am not sure this is worth proving, as it is typically used on ordered lists.

@jamesmckinna
Copy link
Contributor

jamesmckinna commented Apr 25, 2023

Re: 'up to permutation' Is that not what is being captured by _⊆_? (EDITED: maybe I have misunderstood something fundamental here)
Re: the case of totally (linearly) order lists, is that not what is being addressed in the (now re-exported) version in Data.List.Relation.Unary.Sorted.TotalOrder.Properties? In this respect, pace @Taneb 's suggestion to raise it as a separate issue, how hard would it be to prove here, against the background assumptions of the lists being Sorted (EDITED: and hence there being a canonical choice of representative of the permutation equivalence class)?

@andreasabel
Copy link
Member Author

@jamesmckinna wrote:

Re: 'up to permutation' Is that not what is being captured by _⊆_?

No, sublists are order-preserving, so the bigger list just inflates the smaller list by new elements, but the relative order of the old elements stays the same. (Up to permutation would be the "subset" concept for lists.)

Re: the case of totally (linearly) order lists, is that not what is being addressed in the (now re-exported) version in Data.List.Relation.Unary.Sorted.TotalOrder.Properties? In this respect, pace @Taneb 's suggestion to raise it as a separate issue, how hard would it be to prove here, against the background assumptions of the lists being Sorted

I think if the PR is meaningful as such, it should be accepted as such (avoiding feature creep that kills PRs).
More can always be done later.
I don't know how hard it is to prove that the merge is the lub, maybe it is just an inductive proof over the two lists being merged...


module Data.List.Relation.Binary.Sublist.Setoid.Properties
{c ℓ} (S : Setoid c ℓ) where

open import Data.List.Base hiding (_∷ʳ_)
open import Data.List.Relation.Unary.Any using (Any)
open import Data.List.Relation.Binary.Sublist.Heterogeneous using () renaming (minimum to []⊆_)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm best to not rename things unnecessarily? If you wanted to you could add an extra lemma named ⊆-minimum here?

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I'll change, even though it makes the proof less readable, because []⊆ xs is unambiguous for [] ⊆ xs, while minimum xs would (in my intuitition) mean to extract the minimal element from list xs (see Haskell's Foldables).

@MatthewDaggitt
Copy link
Contributor

(I've deleted the Squash me label, as we should always be squashing. For some reason Github's default changed recently, but I've enabled the Linear history option in the settings that should prevent us from not squashing them).

@andreasabel
Copy link
Member Author

(I've deleted the Squash me label, as we should always be squashing. For some reason Github's default changed recently, but I've enabled the Linear history option in the settings that should prevent us from not squashing them).

I thought "linear history" still leaves two options: "rebase" and "squash", the first for well-engineered individual commits and the second for "commit followed by fixups". (So I put up a flag that "rebase" would be a bad option here...)

In my experience, Github's default is whatever you used the last time...

@gallais
Copy link
Member

gallais commented May 12, 2023

Looks good to me. We'll need a CHANGELOG entry though.

@MatthewDaggitt
Copy link
Contributor

@andreasabel any chance you could add a CHANGELOG entry as @gallais requests and then I'll merge this in?

@andreasabel
Copy link
Member Author

@MatthewDaggitt : Sorry, I missed the last comments.
Now there is a CHANGELOG entry.

Btw, why is CHANGELOG.md excluded from fix-whitespace?

@andreasabel
Copy link
Member Author

Fine to merge now, @MatthewDaggitt ?

@MatthewDaggitt
Copy link
Contributor

Btw, why is CHANGELOG.md excluded from fix-whitespace?

Unsure... I guess because it's not code?

@MatthewDaggitt MatthewDaggitt merged commit 8c70a9b into agda:master Jun 20, 2023
@andreasabel andreasabel deleted the merge-is-sublist branch June 20, 2023 06:32
@andreasabel
Copy link
Member Author

andreasabel commented Jun 20, 2023

Thanks for merging!

Btw, why is CHANGELOG.md excluded from fix-whitespace?

Unsure... I guess because it's not code?

Technically, markdown has this obscure (and imo ill-designed) feature that two trailing spaces force a line break (a <br/> tag). https://www.oasys.net/fragments/markdown-line-breaks/
I think one shouldn't make use of this feature, and then enforcing the usual whitespace discipline would be fine.

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.

5 participants