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

[ refactor ] Move Data.List.Relation.Unary.All.Properties.all⊆concat to Data.List.Relation.Binary.Sublist.Setoid.Properties #2524

Merged
merged 14 commits into from
Dec 29, 2024

Conversation

jamesmckinna
Copy link
Contributor

@jamesmckinna jamesmckinna commented Dec 17, 2024

Fixes #2517 following discussion there.
Re-exports to Data.List.Relation.Binary.Sublist.Propositional.Properties automatically, so this strictly generalises #2513 as a solution to #2509 .

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.

Makes me wonder if we have equational proof combinators for ⊆ reasoning.

@jamesmckinna
Copy link
Contributor Author

jamesmckinna commented Dec 18, 2024

Makes me wonder if we have equational proof combinators for ⊆ reasoning.

We do, AFAIK, but would they help here?

Oh, I suppose you could eliminate my go-to pattern of with ... rewrite .... Hmm, I could try that... but I'm still thinking about your comments elsewhere about the fragility of rewrite, and would like to see some concrete examples of such fragility, ie of it working, then breaking. Plenty of examples of it not working, but they seem legitimately to arise from "I'm not sure there is a case for refl when attempting to unify..." diagnostics under cubical-compatible/without-K, which I guess we have to bite the bullet on...

@JacquesCarette
Copy link
Contributor

Will try to collect examples of fragility. My memory is that it mostly arises in two cases: 1) Agda upgrades, 2) (what ought to be) minor refactorings. rewrite is extremely syntatic, and so prone to fail whenever any kind of minor disturbance happens. Of course, some equational proofs also fail in that way! The biggest difference is often rewrite is used to fix up a type, often enough an implicit one, and so why it's failing can be extremely hard to figure out.

@MatthewDaggitt
Copy link
Contributor

I'm with @JacquesCarette on this one. It is also my experience that rewrite proofs tend to be very fragile, and refactoring proofs that use extensive rewriting is extremely painful. In particular, it's very non-obvious which and how many terms in the goal are being rewritten.

To be honest, I would be happy with a blanket policy of no rewrite proofs in the library, but I realise that might be a bit extreme.

@jamesmckinna
Copy link
Contributor Author

jamesmckinna commented Dec 19, 2024

I'll fix up the rewrite step... DONE.
(Turned out to be quite clunky, in lieu of developing ⊆-Reasoning to be able to fold in the equality proof... downstream PR?)

@jamesmckinna
Copy link
Contributor Author

jamesmckinna commented Dec 19, 2024

Makes me wonder if we have equational proof combinators for ⊆ reasoning.

It turns out I was wrong. There is a module ⊆-Reasoning for Subset, but not, it seems, for Sublist. Sigh.
UPDATED: #2527

@jamesmckinna
Copy link
Contributor Author

I'm with @JacquesCarette on this one. It is also my experience that rewrite proofs tend to be very fragile, and refactoring proofs that use extensive rewriting is extremely painful. In particular, it's very non-obvious which and how many terms in the goal are being rewritten.

This perhaps is one of those rare situations where I envy the affordances in the Rocq prover, whereby you can localise a rewrite step to a specific term/hypothesis via rewrite eq in exp... and I think my idiomatic use of it here, precisely only to rewrite in a lemma/subterm introduced by with, is an attempt to simulate that... I won't attempt an argument to suggest that because such a thing is with-bound (and hence the goal is already abstracted) that this idiomatic usage is somehow more robust, because then we'll get into the robustness or otherwise, of with itself... but I suspect that I could enforce that locality further by having prf be defined in a where clause... but I have refactored to avoid all such considerations now, in the spirit of the subsequent:

To be honest, I would be happy with a blanket policy of no rewrite proofs in the library, but I realise that might be a bit extreme.

Well, it's a good yardstick to hold on to, I suppose, and have refactored accordingly. But I agree it might be 'too extreme'... except that we could put it in the style-guide and then not have to relitigate this every time... modulo all the legacy proofs. Sigh.

@jamesmckinna
Copy link
Contributor Author

Given 2 approvals, are you each happy for me also to fold in tightening the imports as identified in #2533 ?

@JacquesCarette
Copy link
Contributor

I'm always pro tighter imports.

@jamesmckinna jamesmckinna linked an issue Dec 24, 2024 that may be closed by this pull request
@JacquesCarette JacquesCarette added this pull request to the merge queue Dec 29, 2024
Merged via the queue into agda:master with commit b429588 Dec 29, 2024
2 checks passed
@jamesmckinna jamesmckinna deleted the issue2517 branch December 29, 2024 18:43
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.

lib-2.1.1: unused import in ...All/Properties.agda [ refactor ] Possible refactoring of #2509 / #2513 ?
3 participants