8000 Insertion sort and its properties. A bug in MergeSort.agda is fixed. by onestruggler · Pull Request #2723 · agda/agda-stdlib · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Insertion sort and its properties. A bug in MergeSort.agda is fixed. #2723

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

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

onestruggler
Copy link

Besides the sorted and permutation peroperties, we also proved a congruence between permutation relation and pointwise relation on lists. And then we prove permuation relation is decidable.

Copy link
Contributor
@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.

This is generally looking really nice. I've only spotted some small improvements that ou 8000 ght to be made, and also left a suggestion.

@jamesmckinna
Copy link
Contributor

So the bug that is fixed is the generalisation from Propositional permutation to Setoid permutation? or something else? I wasn't sure just looking at the diffs.

Copy link
Contributor
@jamesmckinna jamesmckinna left a comment
8000

Choose a reason for hiding this comment

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

Lots of suggestions, but a very nice piece of work.

@onestruggler
Copy link
Author

So the bug that is fixed is the generalisation from Propositional permutation to Setoid permutation? or something else? I wasn't sure just looking at the diffs.

Luckily, the diffs are the only modification to fix the bug mentioned. The bug is: mergesort used a DecTotalOrder to sort, whose underlying equality is not propositional. Before modification, the permutation property only works for a DecTotalOrder with propositional equality, but now any setoid equivalence.

onestruggler and others added 9 commits May 31, 2025 19:14
Co-authored-by: jamesmckinna <31931406+jamesmckinna@users.noreply.github.com>
Co-authored-by: jamesmckinna <31931406+jamesmckinna@users.noreply.github.com>
Co-authored-by: jamesmckinna <31931406+jamesmckinna@users.noreply.github.com>
Co-authored-by: jamesmckinna <31931406+jamesmckinna@users.noreply.github.com>
< 10000 div class="js-details-container Details js-socket-channel js-updatable-content">
Copy link
Contributor
@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.

Sorry, one more thing. This is looking really good now.

}

------------------------------------------------------------------------
-- Congruence properties
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, now I notice that usually such things would be in Data.List.Sort.InsertionSort.Properties rather than directly in the main file.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I agree. Same prolem with Mergesort. Ideally, all properties goes to the "*.Properties" file, such as "Sorted properties" and "Permutation properties" , not only the "Congruence properties". Should we split them? On the other hand, InsertionSort only has less than 200 loc, it seems kinda too dogmatic to split such a small file.

Copy link
Contributor

Choose a reason for hiding this comment

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

There are properties needed to define Algorithm that can stay here, and then the ones after, which should move. But I'm fine with all of them moving.

The file is currently short. But these things have a way of continually expanding. May as well set a good example as early as possible.

Copy link
Author

Choose a reason for hiding this comment

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

agreed and done for two sorting algorithms.

@onestruggler onestruggler changed the title Iinsertion sort and its properties. A bug in MergeSort.agda is fixed. Insertion sort and its properties. A bug in MergeSort.agda is fixed. May 31, 2025
@jamesmckinna
Copy link
Contributor

fix-whitespace needed!


-- But the induction over xs ↭ ys is hard to do for the "transitive"
-- constructor. So instead we have a similar property that depends on
-- the particular sorting algorithm used.
Copy link
Contributor

Choose a reason for hiding this comment

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

As you say in the comment, this isn't really the form of the two lemmas we want to prove. Yes, the transitive constructor makes the proof hard, but its not impossible to do.

I'd really prefer to have the correct lemma in the library, rather than the "wrong" one that we have to go to the faff of removing later...

If the full lemma is too hard to prove, then I suggest we simply remove these two lemmas.

Copy link
Author

Choose a reason for hiding this comment

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

I have tried to prove the full lemma using the current permutation definition, but failed. Issue #2311 started one year ago was trying to fix the definition, but not ready yet.

Ignore the comment for a moment, the property sort-cong-↭ is just a congruence for sort, which itself as a property for sort should be there, but it is implied by the full lemma. Can we change the proof of sort-cong-↭ later and keep this property now? I also have subsequent developement depending on the decidability of permutation. If not, I have no problem removing them.

Copy link
Contributor
@jamesmckinna jamesmckinna Jun 2, 2025

Choose a reason for hiding this comment

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

FWIW, #2311 stalled because of the various discussions around making breaking changes for v3.0. But as an addition with yet another definition, and a proof of equivalence with curent versions, sure I can add one.

Incoming... #2726

Copy link
Contributor

Choose a reason for hiding this comment

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

@onestruggler I've proved the general lemma

↗↭↗⇒≋ :  {xs ys}  Sorted O xs  Sorted O ys  xs ↭ ys  xs ≋ ys

in the branch sorting-perm-equality2. A PR will be incoming in the next few days, so can you remove these lemmas from your PR. You can then add any extra results you need based on the new lemma in a new PR?

Copy link
Author

Choose a reason for hiding this comment

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

The "property1 : ∀ {xs ys} → xs ~ ys → Sorted xs → Sorted ys → xs ≋ ys" is still hard to prove using the new definition in #2726 . Induction over ⋎ cannot go through, becasue I need "b ∷ cs" to be sorted in order to call IH on it. It is unreasonable to ask "b ∷ cs" to be sorted.

Copy link
Author

Choose a reason for hiding this comment

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

If you squint at the following line in the new def of permuation in #2726

: as ∼ b ∷ cs → a ∷ cs ∼ bs → a ∷ as ∼ b ∷ bs

this line is essentially transitivity (plus swap): as ~ cs -> cs ~ bs -> as ~ bs.

The reason we cannot get rid of transitivity is that transitivity is composition of permutations. We cannot list all N! permutations in our inductive data type, we can only choose some generators, e.g. right now all swaps (transpositions) (we know transpositions generates all permuations) , so we have to compose generators to get all permutations. So transitivity is non-elimable. So induction proof can never avoid transivity, hence hard to go through.

This suggests the "full lemma" as called by Matt is hard to prove using whatever defs of permutations. Hence the not-so-full lemma depending on a particular sorting algorithm seems the only way to do it.

Copy link
Author
@onestruggler onestruggler Jun 3, 2025

Choose a reason for hiding this comment

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

@onestruggler I've proved the general lemma

↗↭↗⇒≋ :  {xs ys}  Sorted O xs  Sorted O ys  xs ↭ ys  xs ≋ ys

in the branch sorting-perm-equality2. A PR will be incoming in the next few days, so can you remove these lemmas from your PR. You can then add any extra results you need based on the new lemma in a new PR?

Yes, I am going to make a new PR later. Your proof is impressive! Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

@onestruggler thanks for the comments on #2726 suggest we move the discussion there? But yes, the merge constructor indeed incorporates some transitivity, but in a more constrained way, moreover list cs is shorter than as or bs (which drives the proof of full transitivity)

Copy link
Author

Choose a reason for hiding this comment

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

Hi @jamesmckinna , sure, let's move to #2726 after this post. Matt has a proof that avoids induction over transitivity, so different defs for permutation is no longer urgent for proving the "full lemma". But simple defs is definitely preferable.

@MatthewDaggitt MatthewDaggitt added this to the v2.3 milestone Jun 2, 2025
@MatthewDaggitt
Copy link
Contributor

Also needs a CHANGELOG entry.

Copy link
Contributor
@MatthewDaggitt MatthewDaggitt left a comment

Choose a reason for hiding this comment

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

Ah also I've just realised a really crucial point! The permutations were propositional based because that is actually the stronger result than them being setoid based permutations!

Please can you revert the changes back so that they are propositional permutations! (and of course remove that statement from the CHANGELOG!)

@MatthewDaggitt
Copy link
Contributor

It might be good to add a comment as well explaining why the proofs are propositional based. Note that it is very easy to convert from propositional permutation to setoid-based permutation (but not the other way around!)

@onestruggler
Copy link
Author
onestruggler commented Jun 3, 2025

Ah also I've just realised a really crucial point! The permutations were propositional based because that is actually the stronger result than them being setoid based permutations!

Please can you revert the changes back so that they are propositional permutations! (and of course remove that statement from the CHANGELOG!)

I must misundertand something. I think setoid-based sorting is more general (stronger) than propositional based. If you want to sort propositionally you just use a DecTotalOrder whose underlying equivalence is propositional. Then you use the following to get to the "real propsitional":

But if you only have propositional sorting, you cannot sort setoidly.

Every propositional equality is a setoid. You can always turn propostional equaltiy into a setoid equivalence by refl but not visa vera, that is setoid is more general.

@MatthewDaggitt
Copy link
Contributor
MatthewDaggitt commented Jun 4, 2025

I must misundertand something. I think setoid-based sorting is more general (stronger) than propositional based.

The crucial thing is that sorting should preserve the elements in the list right? Even if you are using a setoid based DecTotalOrder, you still want to enforce that the sorting algorithm should not be allowed to actually mutate the elements (even if the mutation is equal under the setoid-based equality.)

For example, imagine sorting unnormalised rationals quotiented over by the setoid in the obvious way (e.g. 1/2 = 2/4 in the setoid). If you say that sorting only needs to return a setoid-based permutation then legally sort [1/3,1/2] could return [1000001/2000002, 123/369]. If you saw this, then you'd think the sorting algorithm had gone very wrong right?

Crucially, it's actually impossible for the sorting algorithm to do that as the way we have created the interface, it is impossible to mutate the elements of the list at all as the interface is entirely generic as to the carrier set being passed in.

@jamesmckinna
Copy link
Contributor

Very interesting discussions... who knew insertion sort still had things to teach us!?

@onestruggler
Copy link
Author

I must misundertand something. I think setoid-based sorting is more general (stronger) than propositional based.

The crucial thing is that sorting should preserve the elements in the list right? Even if you are using a setoid based DecTotalOrder, you still want to enforce that the sorting algorithm should not be allowed to actually mutate the elements (even if the mutation is equal under the setoid-based equality.)

Yes, it should preserve elements. But if I used a setoid based DecTotalOrder with the setoid being propostional equality, it also never changes elements.

For example, imagine sorting unnormalised rationals quotiented over by the setoid in the obvious way (e.g. 1/2 = 2/4 in the setoid). If you say that sorting only needs to return a setoid-based permutation then legally sort [1/3,1/2] could return [1000001/2000002, 123/369]. If you saw this, then you'd think the sorting algorithm had gone very wrong right?

In a setoid, 1/3 is 123/369, the return looks good to me. Also, at least for insertion sort, it does not change the elements whatsoever: it compares and puts everything back.

The old interface cannot sort unnormalised rationals (because the underlying equivalence is non-propositional equivalence), but can sort normalised rationals (because the underlying equivalence is propositional equality), while the new sort can sort both. When the new one sorts normalised rationals, it behaves the same as the old one (guarantted by ↭ₛ⇒↭ : ↭ₛ ). That is the new one is stronger.

I need to hear more voices on this: @JacquesCarette @jamesmckinna @larrytheliquid

@jamesmckinna
Copy link
Contributor

I'm now in meetings for the rest of the day, but lots of food for thought regarding Propositional vs. Setoid representations wrt stability of sorting... and having not really thought about these things since my PhD thesis (1988--92 ;-)) I need to scratch my head a bit before saying anything stupid here...

@MatthewDaggitt
Copy link
Contributor

Yes, it should preserve elements. But if I used a setoid based DecTotalOrder with the setoid being propostional equality, it also never changes elements.

This is beside the point.

In a setoid, 1/3 is 123/369, the return looks good to me.

Really, a sorting algorithm that magically changes 1/3 to 123/369 looks good to you?

Also, at least for insertion sort, it does not change the elements whatsoever: it compares and puts everything back.

Yes, exactly! That's my point! It's impossible to write a generic sorting algorithm that changes the elements. Hence this should be reflected in the type signature.

The old interface cannot sort unnormalised rationals (because the underlying equivalence is non-propositional equivalence), but can sort normalised rationals (because the underlying equivalence is propositional equality), while the new sort can sort both. When the new one sorts normalised rationals, it behaves the same as the old one (guarantted by ↭ₛ⇒↭ : ↭ₛ ⇒ ↭ ). That is the new one is stronger.

This is not true. With the current interface you can pass in the total order over unnormalised rationals which uses the setoid-based equality and then derive both the setoid and the non-setoid permutation result:

open import Data.Rational.Unnormalised
open import Data.Rational.Unnormalised.Properties
open import Data.List
open import Data.List.Sort ≤-decTotalOrder
import Data.List.Relation.Binary.Permutation.Homogeneous as Homo
import Data.List.Relation.Binary.Permutation.Setoid ≃-setoid as Setoid
import Data.List.Relation.Binary.Permutation.Propositional as Prop

l : List ℚᵘ
l = 1ℚᵘ ∷ 0ℚᵘ ∷ []

sl : List ℚᵘ
sl = sort l

sorted-prop-↭ : sl Prop.↭ l
sorted-prop-↭ = sort-↭ l

sorted-setoid-↭ : sl Setoid.↭ l
sorted-setoid-↭ = Homo.map ≃-reflexive (Prop.↭⇒↭ₛ (sorted-prop-↭))

So challenge for you. Can you derive sorted-prop-↭ after your alterations while keeping the imports identical (i.e. while passing in ≤-decTotalOrder as the argument to List.Sort)?

@onestruggler
Copy link
Author

Yes, exactly! That's my point! It's impossible to write a generic sorting algorithm that changes the elements. Hence this should be reflected in the type signature.

I finally see your point. Thanks for the clarification. I indeed misunderstood it. I apologize for being stupid.

In some sense, for sorting, the setoid equivalence in a DecTotalOrder is never used, in other words, in some sense, in the eyes of sorting, any DecTotalOrder is always propostional-based. (the sentence before is just a vague idea, please do not take it literally). So restricting to propostional permutation loses nothing and is indeed stronger than setoid permutation (as the challenge shows, you can never recover propostional equality from setoid equivalence).

This should be documented in case of someone like me misunderstand it again.

@JacquesCarette
Copy link
Contributor

Glad it was sorted (ahem) out. Yes, this is super subtle.

Happy to help document -- but where? I guess it doesn't matter as much as to where we put pointers to such documentation, so that you might have seen it while doing this work.

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.

4 participants
0