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

Data.Nat.Properties.≤-total should be implemented with _<ᵇ_ #2436

Open
Taneb opened this issue Jul 11, 2024 · 14 comments · May be fixed by #2440
Open

Data.Nat.Properties.≤-total should be implemented with _<ᵇ_ #2436

Taneb opened this issue Jul 11, 2024 · 14 comments · May be fixed by #2440

Comments

@Taneb
Copy link
Member

Taneb commented Jul 11, 2024

It's very slow for ≤-total to decide which constructor to use, especially compared to ≤?. This is an issue using Data.Nat with code generic over a TotalOrder - it's far slower than it could be.

@jamesmckinna
Copy link
Contributor

Given your comment about ≤?, is there an alternative, but still generic, implementation of ≤-total going via Decidable _≤_ rather than TotalOrder?

@Taneb
Copy link
Member Author

Taneb commented Jul 15, 2024

@jamesmckinna I don't understand your question. I noticed this problem when I wrote some code generic over a TotalOrder (a record containing a proof that an order is total, i.e. every pair of elements is comparable - ≤-total witnesses this for ). Data.Nat.Properties exports ≤-totalOrder, an instance of this record, using ≤-total.

We could define ≤-total in terms of _≤?_ and ≰⇒≥, but this requires some annoying reordering of Data.Nat.Properties. Here's what this might look like:

≤-total : Total _≤_
≤-total m n with m ≤? n
... | yes m≤n = inj₁ m≤n
... | no m≰n = inj₂ (≰⇒≥ m≰n)

@gallais
Copy link
Member

gallais commented Jul 15, 2024

We could define ≤-total in terms of _≤?_ and ≰⇒≥, but this requires some annoying reordering of Data.Nat.Properties.

I think that was the suggestion, and I think it's worth the reorganisation.

@jamesmckinna
Copy link
Contributor

Yes, sorry if my comment was too oblique... @gallais is correct. Cf. my v2.0 refactorings of Data.Nat.DivMod in #2182 ...

Moreover, the proof you offer, modulo the equivalence of _≤?_ and _<ᵇ_, is morally the/a reimplementation of the issue's title, or not?

But I did even wonder if such a (re-)definition should moreover be under Relation.Binary.* (somewhere as Properties or Consequences of being a DecPoset?), and then that proof be instantiated in Nat-specific terms?

Not sure why the reordering of Data.Nat.Properties should be so annoying, other than the fact that refactoring/reimplementation might have such dependency reordering as a knock on consequence?

@jamesmckinna
Copy link
Contributor

@gallais does this qualify as a bug though?

@JacquesCarette
Copy link
Contributor

JacquesCarette commented Jul 15, 2024

I'm not sure how much we should change things wrt efficiency. What I mean is that it makes a lot of sense to have (at least) two different definitions of many concepts, one 'conceptually clean' and one 'efficient'. There's quite a number of things in (for example) Data.Rational that would be better off existing in two version. A number already do.

I think it's a false choice to "choose" between certain definitions. Rather we should have parallel hierarchies of definitions (and equivalence proofs). I'd be happy with, say, Data.Rational.Efficient (and same for Data.Integer and likely many more).

There was a reason for that paper on "Realms"!!

@Taneb
Copy link
Member Author

Taneb commented Jul 16, 2024

@JacquesCarette in this case, there shouldn't be anything downstream of ≤-total depending on its implementation. If we provide a choice between ≤-total and ≤-total-fast, there'd be no reason for a user to use ≤-total over ≤-total-fast, other than perhaps avoiding a single extra import. I'd be more inclined to accept your argument in cases where we do prove properties using specific implementation details (such as Data.Nat.Base._⊔_/_⊔′_, where we do exactly that)

@gallais
Copy link
Member

gallais commented Jul 17, 2024

I would be strongly against -fast suffixes: the fast version should IMO
be the default and we can have a -spec variant for the equivalent but
more proof-friendly version.

@Taneb
Copy link
Member Author

Taneb commented Jul 17, 2024

I wasn't advocating using a -fast suffix! I was trying to argue against having two different versions at all

@jamesmckinna
Copy link
Contributor

Again, re:labelling this issue, not sure why this is a bug, while at the same time arguing for improved reimplementation is not refactoring???

@gallais
Copy link
Member

gallais commented Jul 18, 2024

Ah sorry, I originally labeled it a refactoring before re-labeling it a performance bug and removing the
refactoring label. I had not noticed you had added it back and so assumed my shaky connection meant that
github had not taken my attempted update into account.

@Taneb Taneb linked a pull request Jul 19, 2024 that will close this issue
@jamesmckinna
Copy link
Contributor

jamesmckinna commented Jul 26, 2024

@gallais 's comment on the PR #2440 suggests that this would be a breaking change...(wrt intensional/reduction behaviour), so: v3.0?

@JacquesCarette
Copy link
Contributor

So while I was probably misguided to rant about spec vs fast on this particular issue, it still seems that there is an issue (but with reduction behaviour). Though I must admit that I'm getting increasingly sad when we do have code that does that (depend on reduction behaviour). Having said that, I do understand that Agda may not be quite ready to deal elegantly with implementation-obliviousness.

@jamesmckinna
Copy link
Contributor

@JacquesCarette writes:

... Though I must admit that I'm getting increasingly sad when we do have code that does that (depend on reduction behaviour). Having said that, I do understand that Agda may not be quite ready to deal elegantly with implementation-obliviousness.

I absolutely agree, and/but what I'm not clear about is whether we should be more diligent as contributors/reviewers in trying to enforce this: eg I'm increasingly fastidious about replacing uses of ⊥-elim and related things (which exploit the definition of negation as an implication, and hence privilege function application as an implicit short-cut definition) in favour of explicit appeals to contradiction etc.

A long time ago, I read a very good piece (where? google can't seem to help me find this... a mailing list somewhere?) by Frank Atanassow explicitly critiquing the use of dependent types in programming precisely because of the almost inevitable temptation/necessity of exposing implementations in types...

... and even longer ago, two authors argued for the use of dependent types in programming precisely because of its ability to (better!) support Wadler's idea of allowing pattern matching to cohabit with data abstraction, not least because such analyses tend/have tended to focus on extensional behaviour (wrt reduction) rather than complexity-sensitive intensional behaviour... so it seems we still have a long way to go ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants