Skip to content

Proved sorted permutations are equal #2748

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

Merged
merged 3 commits into from
Jul 2, 2025
Merged

Conversation

MatthewDaggitt
Copy link
Contributor

The main result. Would like to try and sneak this into v2.3 for @onestruggler.

Have also added an explanation of the choice of propositional permutation in SortingAlgorithm and a lemma that proves the setoid version.

Copy link
Contributor

@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.

Various nitpicks and bugfixes suggested, but otherwise: good to go!

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.

I have no comments beyond James'.

Copy link
Contributor

@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.

Sorry to go back on my original approval, but I think the new proof structure is... better!?

@JacquesCarette
Copy link
Contributor

Definitely agree that your proof of injective⇒existsPivot is much nicer.

Minor nit: f∘inject! instead of h? Then the 'narrative' of the proof would be easier to follow top-down, instead of being forced to reconstruct it by reading it bottom-up.

@jamesmckinna
Copy link
Contributor

@JacquesCarette if @MatthewDaggitt goes ahead with my proposed changes, then I think your function name change makes sense, but perhaps at the cost of what you then choose to call what currently is h-injective...so I'll leave such last things up to him... ;-)

@jamesmckinna
Copy link
Contributor

Definitely agree that your proof of injective⇒existsPivot is much nicer.

Minor nit: f∘inject! instead of h? Then the 'narrative' of the proof would be easier to follow top-down, instead of being forced to reconstruct it by reading it bottom-up.

lower∘f∘inject!? I'd almost be tempted to call it ↓f↑? Or... f′ for 'f modified'?

@JacquesCarette
Copy link
Contributor

I think lower gets elided a lot, and that's fine with me. So f∘inject! seems to capture the important meaning.

Copy link
Contributor

@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.

All looks great.
Last round of nitpicks: CHANGELOG is the one that really needs fixing

@MatthewDaggitt MatthewDaggitt enabled auto-merge July 2, 2025 07:19
@jamesmckinna
Copy link
Contributor

🎉 Very nice work!

@MatthewDaggitt MatthewDaggitt added this pull request to the merge queue Jul 2, 2025
Merged via the queue into master with commit 5bd195e Jul 2, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants