Skip to content

Change in definition of Data.List.Base.filter etc. result in new unsolved metas in proofs #2172

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

Closed
MatthewDaggitt opened this issue Oct 20, 2023 · 8 comments · Fixed by #2186
Assignees
Milestone

Comments

@MatthewDaggitt
Copy link
Contributor

The definition of filter etc got changed to be defined in terms of the boolean test rather than a decidable predicate. Several problems have resulted:

  1. Firstly this is not documented in the CHANGELOG
  2. This has lead to many of the proofs not being able to infer their list argument. In particular filter-accept and filter-reject in Data.List.Properties now need their xs parameter to be passed explicitly.

We should check all functions which have been redefined similarly.

@MatthewDaggitt MatthewDaggitt added this to the v2.0 milestone Oct 20, 2023
@jamesmckinna
Copy link
Contributor

jamesmckinna commented Oct 20, 2023

git blame offers this commit as the offending one, which seems to have touched all the functions in the same way.

@jamesmckinna
Copy link
Contributor

Do you have examples of eg failing client code/tests? The usual make test cycle (locally, and on GitHub) doesn't show this issue up, it seems.

@MatthewDaggitt
Copy link
Contributor Author

MatthewDaggitt commented Oct 21, 2023

For example, these two are failing in my own code base:

https://github.com/MatthewDaggitt/agda-routing/blob/ee065c039ee0b677ff5651f433d2dcf20311d044/RoutingLib/lmv34/Synchronous/Gamma_one/Properties.agda#L230

https://github.com/MatthewDaggitt/agda-routing/blob/ee065c039ee0b677ff5651f433d2dcf20311d044/RoutingLib/lmv34/Synchronous/Gamma_one/Properties.agda#L233

The usual make test cycle (locally, and on GitHub) doesn't show this issue up, it seems.

Yes, I suspect we don't actually ever use any of the filter lemmas in the library? I can't think of anywhere we do.

@jamesmckinna
Copy link
Contributor

jamesmckinna commented Oct 21, 2023

Indeed, there are no uses in the library. I'm curious as to how they might have typechecked under the old definitions, unless those had explicit quantification, and somewhere along the line, we made the explicit->implicit 'mistake'...?

A better/more precise question: what is the last version of the library that those proofs did check under?

@MatthewDaggitt MatthewDaggitt self-assigned this Oct 30, 2023
@MatthewDaggitt
Copy link
Contributor Author

Ah I remember why I defined the predicate versions in terms of the boolean ones. Because in order to convert from boolean to decidable predicates you need to use T? from Data.Bool.Properties in Data.List.Base, which violates our dependency management convention that we never import Properties modules from Base modules....

@jamesmckinna
Copy link
Contributor

jamesmckinna commented Oct 30, 2023

Well... see previous discussion about moving that operator to Relation.Nullary.Decidable.Core... ;-)

@jamesmckinna
Copy link
Contributor

jamesmckinna commented Nov 1, 2023

Re: your opening comment

Has the behaviour of filter-accept and filter-reject now reverted to the previous (implicit quantification) behaviour as a consequence of #2186 ?

Apologies: I suppose that it must be the case, because the tests pass (sic)...?

@MatthewDaggitt
Copy link
Contributor Author

Yes, that is the case 👍

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.

2 participants