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

[hover] Display debug statistic for universes #666

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ejgallego
Copy link
Owner

@ejgallego ejgallego commented Apr 4, 2024

Stats seem a bit mixed, more research is needed.

We will improve the analysis shortly, for now we do two kind of analysis:

  • on hover, we print the number of universes and constraints at point
  • using fcc --root $abs_path --plugin=coq-lsp.plugin.univdiff $abs_file.v , the plugin will output an $abs_file.v.univdiff file with the following information:
    • a line univs for "sentence": (nunivs, nconstr) { +diff_nunivs, +diff_constr } for "sentence"s that have changed the number of universes
    • a summary for qed items, where qed_total: n is the number of qed in the file, and qed_yes: n is the number of qed that actually changes the universe constraints.

@ejgallego ejgallego modified the milestones: 0.1.9, 0.1.10 Apr 4, 2024
@ejgallego
Copy link
Owner Author

@SkySkimmer find some preliminary results on Qed universe alterations:

- ssrnat.v: qed_total: 479 / qed_yes: 10
- seq.v: qed_total: 790 / qed_yes: 179
- bigop.v: qed_total: 295 / qed_yes: 137
- compcert/lib/Coqlib: qed_total: 114 / qed_yes: 70
- compcert/lib/Integers: qed_total: 404 / qed_yes: 3

typical profile for ssrnat.v:

univs for "From mathcomp Require Import ssreflect ssrfun ssrbool eqtype.":
 ( 751,3806) {+344, +1802}
univs for "Require Import BinNat.":
 (1017,4914) {+266, +1108}
univs for "Require BinPos Ndec.":
 (1373,7080) {+356, +2166}
univs for "Qed.":
 (1374,7084) {+1, +4}
univs for "Qed.":
 (1374,7085) {+0, +1}
univs for "Qed.":
 (1375,7088) {+1, +3}
univs for "Inductive ex_minn_spec : nat -> Type :=
  ExMinnSpec m of P m & (forall n, P n -> n >= m) : ex_minn_spec m.":
 (1376,7090) {+1, +2}
univs for "Variable T : Type.":
 (1377,7092) {+1, +2}
univs for "Qed.":
 (1377,7093) {+0, +1}
univs for "Qed.":
 (1377,7094) {+0, +1}
univs for "Qed.":
 (1377,7095) {+0, +1}
univs for "Qed.":
 (1377,7100) {+0, +5}
univs for "Qed.":
 (1378,7106) {+1, +6}
univs for "Qed.":
 (1379,7113) {+1, +7}
univs for "Variable T : Type.":
 (1380,7115) {+1, +2}
univs for "Qed.":
 (1380,7116) {+0, +1}

@ejgallego ejgallego force-pushed the hover_univdiff branch 3 times, most recently from 410e6f7 to ae5cbe9 Compare April 5, 2024 01:11
@SkySkimmer
Copy link
Collaborator

How do I read those results?

@ejgallego
Copy link
Owner Author

How do I read those results?

Added some more info to the PR description. Will add some more info and an improved hover mode.

I did some more experiments in Compcert for example and the results are not too bad, we should be able to skip a lot of proofs and re-check them, tho still the dynamic barriers can be complex to schedule in parallel, but the linear model doesn't look too bad.

Something interesting is that some Qed happen to remove constraints.

@SkySkimmer
Copy link
Collaborator

Something interesting is that some Qed happen to remove constraints.

Probably just collapsing some loops? eg if you have a <= b <= c and a <= c (3 constraints) then add c <= a the graph becomes a = b = c (2 constraints)

@ejgallego
Copy link
Owner Author

Something interesting is that some Qed happen to remove constraints.

Probably just collapsing some loops? eg if you have a <= b <= c and a <= c (3 constraints) then add c <= a the graph becomes a = b = c (2 constraints)

Interesting! I'm gonna add some code to "diff" universe graphs so we can see with more details what is going on.

Feel free to push to this branch if you want to add some kind of diff, or let me know if we already have some functions that could be useful.

Note that I do Global.universes () so we are not getting the new constraints added in tactics, maybe we would like to correct this too?

@SkySkimmer
Copy link
Collaborator

See also coq/coq#18900

@ejgallego
Copy link
Owner Author

Cool!

The way I'd go ahead with this PR is to have the plugin emit a warning when the diff is not 0, and in some cases display the diff (for example not on require)

That should be a good way to debug rebuilds as soon as we start skipping Qed proofs here.

@SkySkimmer
Copy link
Collaborator

Does your debug info say +1 univ on something like

Lemma foo : forall A : Type, A -> A.
Proof. intros A x;exact x. Qed.

?

@ejgallego
Copy link
Owner Author

ejgallego commented Apr 5, 2024

Lemma foo : forall A : Type, A -> A.
Proof. intros A x;exact x. Qed.

It says

univs for "Qed.":
 ( 408,2006) {+1, +2}
qed_total: 1 / qed_yes: 1

so yes.

Note that I don't restore the initial env at Qed time as is done in the Stm, so that's a difference that could matter.

Stats seem a bit mixed, more research is needed.

We will improve the analysis shortly, by output diffs and summaries.
Can be used with

```
$ dune exec -- fcc --root $absolute_path --plugin=coq-lsp.plugin.univdiff $absolute_path.v
$ cat $absolute_path.v.univdiff
```

Attention to compiling files with error-recovery, that can lead to
misleading results.
@Alizter
Copy link
Collaborator

Alizter commented May 16, 2024

May I suggest using the heatmap for this?

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

Successfully merging this pull request may close these issues.

None yet

3 participants