change computation of uniform parameters#21498
change computation of uniform parameters#21498thomas-lamiaux wants to merge 1 commit intorocq-prover:masterfrom
Conversation
6c67c07 to
9b30b9e
Compare
|
@coqbot run full ci |
|
🔴 CI failure at commit 9b30b9e without any failure in the test-suite ✔️ Corresponding job for the base commit 80ca8f9 succeeded ❔ Ask me to try to extract a minimal test case that can be added to the test-suite 🏃
|
|
Can you fix the generator code without modifying the kernel? |
|
This can make rocq take arbitrary amounts of time, eg #[local] Set Warnings "-abstract-large-number".
Definition size := 0x100000000.
Record Fin (n : nat) := Fin_mk { val : nat; isLt : (val < n) }.
Record UInt32 := mk { val0 : Fin size }. |
|
🔴 CI failure at commit 9b30b9e without any failure in the test-suite ✔️ Corresponding job for the base commit 80ca8f9 succeeded ❔ Ask me to try to extract a minimal test case that can be added to the test-suite 🏃
|
| [register-all,automation,default] | ||
| File "./output/Inductive.v", line 62, characters 0-74: | ||
| Warning: Cut is nested using Box. No scheme for Box is registered as All. | ||
| [register-all,automation,default] |
There was a problem hiding this comment.
why is this warning getting printed 12 times?
There was a problem hiding this comment.
No idea, it should be printing four times, once per induction scheme because they are a look up for Box each time
There was a problem hiding this comment.
It is probably because the type is generated, and the term, and the term needs to generate the type, and it ends with a x3
There was a problem hiding this comment.
but this was already the case for nested_eliminators.out
9b30b9e to
52e2222
Compare
It needs to stay for different reasons, plus, it is another job to start refactoring the kernel than fixing a bug |
|
@coqbot run full ci |
52e2222 to
e747e7a
Compare
|
@coqbot run full ci |
e747e7a to
c188a86
Compare
|
@coqbot run full ci |
It's only a bug because your generator assumed a semantics that was not the one computed here, the rest of the code seems fine. You have another solution which is to not trust this number and compute it with your own semantics on your side. |
|
|
@yannl35133, @SkySkimmer and @ppedrot agreed to do this change, if you oppose as a maintainer of the kernel, or they are convinced by your arguments, it can be discussed next Rocq call. As you wish. |
|
@coqbot bench |
Adapt to rocq-prover/rocq#21498 (changed computation uniform parameters)
Examples that get rejected after the PR: Variant Empty (A : Type) := .
Variant Box (F : Type -> Type) (t : Type) := box (x : F t).
Inductive Weird1 (A : Type) : Type :=
| weird1 : A -> Empty (Weird1 (Weird1 (A * A))) -> Weird1 A.
Inductive Weird2 (A : Type) : Type :=
| weird2 : A -> Box Weird2 A -> Weird2 A.
Inductive Rec1 := C1 (x : Weird1 Rec1).
Inductive Rec2 := C2 (x : Weird2 Rec2).
Fixpoint f1 (x : Rec1) : False := match x with C1 (weird1 _ x _) => f1 x end.
Fixpoint f2 (x : Rec2) : False := match x with C2 (weird2 _ x _) => f2 x end. |
Not used, not needed => bug
Not an argument. At the opposite, only having to change 10l out of the whole CI shows that people only stumble on it by accident
It is computation is perfectly sensible. Moreover, they are no spec of the computation of uniform parameters in the refman, which we can add with this PR.
I don't see the issue with this example no longer be accepted. Variant Empty (A : Type) := .
Inductive Weird1 (A : Type) : Type :=
| weird1 : A -> Empty (Weird1 (Weird1 (A * A))) -> Weird1 A.
Fail Inductive Rec1 := C1 (x : Weird1 Rec1).At the opposite, rejecting it is a feature. The following example is the only one that can break and needs fixing. Variant Box (F : Type -> Type) (t : Type) := box (x : F t).
Inductive Weird2 (A : Type) : Type :=
| weird2 : A -> Box Weird2 A -> Weird2 A.but as explained it is equi-expressive to the following, into which it can easily be rewritten. Variant Box (F : Type) := box (x : F).
Inductive Weird2 (A : Type) : Type :=
| weird2 : A -> Box (Weird2 A) -> Weird2 A. |
|
@yannl35133 @tabareau It actually does change something non-trivial if you start hiding stuff under match like this type Inductive depends_on_b (b : bool) (F : Type -> Type) (A : Type) :=
| cdepb : if b then F nat else F A -> depends_on_b b F AIf you define the following Inductive RT A :=
| node : depends_on_b false RT AThen you can't nest on |
|
@yannl35133 what could be done is keep the former value, and use my code to raise an error at definition if it finds a smaller value saying it may cause the generation of elimination principles to fail ? Note this is linked to #21495 |
|
🏁 Bench results: 🐢 Top 25 slow downs┌──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐ │ TOP 25 SLOW DOWNS │ │ │ │ OLD NEW DIFF %DIFF Ln FILE │ ├──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤ │ 96.2 97.6 1.3503 1.40% 968 coq-performance-tests-lite/src/fiat_crypto_via_setoid_rewrite_standalone.v.html │ │ 11.5 12.2 0.7100 6.18% 194 coq-fiat-crypto-with-bedrock/src/Fancy/Barrett256.v.html │ │ 1.23 1.79 0.5610 45.55% 813 rocq-stdlib/theories/MSets/MSetRBT.v.html │ │ 27.2 27.7 0.5125 1.89% 148 coq-fiat-crypto-with-bedrock/src/Bedrock/End2End/X25519/GarageDoorTop.v.html │ │ 46.6 47.1 0.4216 0.90% 115 coq-bedrock2/bedrock2/src/bedrock2Examples/full_mul.v.html │ │ 11.8 12.2 0.4121 3.50% 324 coq-unimath/UniMath/CategoryTheory/Hyperdoctrines/PartialEqRels/Logic/Existential.v.html │ │ 0.248 0.626 0.3772 151.88% 34 rocq-stdlib/theories/Logic/SetoidChoice.v.html │ │ 24.6 25.0 0.3699 1.50% 12 coq-fourcolor/theories/proof/job319to322.v.html │ │ 11.7 12.0 0.3535 3.03% 388 coq-unimath/UniMath/CategoryTheory/Hyperdoctrines/PartialEqRels/Logic/Existential.v.html │ │ 46.6 46.9 0.3311 0.71% 115 coq-fiat-crypto-with-bedrock/rupicola/bedrock2/bedrock2/src/bedrock2Examples/full_mul.v.html │ │ 4.02 4.32 0.2996 7.45% 492 rocq-stdlib/theories/Reals/Cauchy/ConstructiveCauchyRealsMult.v.html │ │ 29.8 30.1 0.2947 0.99% 12 coq-fourcolor/theories/proof/job001to106.v.html │ │ 18.0 18.3 0.2856 1.59% 32 coq-performance-tests-lite/src/pattern.v.html │ │ 29.0 29.3 0.2793 0.96% 145 coq-fiat-crypto-with-bedrock/src/Bedrock/End2End/X25519/GarageDoorTop.v.html │ │ 0.346 0.622 0.2765 79.92% 586 rocq-stdlib/theories/Strings/Byte.v.html │ │ 29.3 29.6 0.2648 0.90% 12 coq-fourcolor/theories/proof/job589to610.v.html │ │ 0.326 0.590 0.2636 80.85% 1 rocq-stdlib/theories/ZArith/ZNsatz.v.html │ │ 0.261 0.518 0.2577 98.89% 19 rocq-stdlib/theories/FSets/FSetFacts.v.html │ │ 6.79 7.03 0.2473 3.64% 842 coq-fiat-crypto-with-bedrock/src/Bedrock/End2End/X25519/GarageDoor.v.html │ │ 28.7 28.9 0.2387 0.83% 12 coq-fourcolor/theories/proof/job563to588.v.html │ │ 7.61 7.85 0.2369 3.11% 633 coq-unimath/UniMath/CategoryTheory/Hyperdoctrines/PartialEqRels/Logic/Existential.v.html │ │ 0.274 0.506 0.2320 84.74% 13 rocq-stdlib/theories/ZArith/Zmin.v.html │ │ 0.306 0.530 0.2234 72.91% 11 rocq-stdlib/theories/setoid_ring/Rings_Q.v.html │ │ 120 120 0.2229 0.19% 22 coq-fiat-crypto-with-bedrock/src/Rewriter/Passes/ArithWithCasts.v.html │ │ 20.8 21.0 0.2226 1.07% 479 rocq-metarocq-erasure/erasure/theories/EWcbvEvalCstrsAsBlocksFixLambdaInd.v.html │ └──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘ 🐇 Top 25 speed ups┌──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐ │ TOP 25 SPEED UPS │ │ │ │ OLD NEW DIFF %DIFF Ln FILE │ ├──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤ │ 67.6 62.7 -4.9549 -7.33% 608 coq-bedrock2/bedrock2/src/bedrock2Examples/lightbulb.v.html │ │ 67.6 64.7 -2.8590 -4.23% 608 coq-fiat-crypto-with-bedrock/rupicola/bedrock2/bedrock2/src/bedrock2Examples/lightbulb.v.html │ │ 203 201 -2.4097 -1.19% 8 coq-neural-net-interp-computed-lite/theories/MaxOfTwoNumbersSimpler/Computed/AllLogits.v.html │ │ 40.5 38.8 -1.6610 -4.10% 1423 coq-fiat-crypto-with-bedrock/rupicola/bedrock2/compiler/src/compiler/FlatToRiscvFunctions.v.html │ │ 44.7 43.4 -1.3687 -3.06% 578 coq-fiat-crypto-with-bedrock/rupicola/bedrock2/compiler/src/compiler/MMIO.v.html │ │ 31.5 30.5 -0.9931 -3.15% 139 coq-fiat-crypto-with-bedrock/src/Bedrock/Field/Synthesis/Examples/p224_64_new.v.html │ │ 31.6 30.6 -0.9813 -3.11% 157 coq-fiat-crypto-with-bedrock/src/Bedrock/Field/Synthesis/Examples/p224_64_new.v.html │ │ 32.8 31.8 -0.9752 -2.97% 121 coq-fiat-crypto-with-bedrock/src/Bedrock/Field/Synthesis/Examples/p224_64_new.v.html │ │ 31.7 30.8 -0.9578 -3.02% 148 coq-fiat-crypto-with-bedrock/src/Bedrock/Field/Synthesis/Examples/p224_64_new.v.html │ │ 31.6 30.8 -0.8258 -2.61% 180 coq-fiat-crypto-with-bedrock/src/Bedrock/Field/Synthesis/Examples/p224_64_new.v.html │ │ 31.5 30.7 -0.8212 -2.61% 166 coq-fiat-crypto-with-bedrock/src/Bedrock/Field/Synthesis/Examples/p224_64_new.v.html │ │ 31.4 30.6 -0.8041 -2.56% 198 coq-fiat-crypto-with-bedrock/src/Bedrock/Field/Synthesis/Examples/p224_64_new.v.html │ │ 31.6 30.8 -0.8021 -2.54% 214 coq-fiat-crypto-with-bedrock/src/Bedrock/Field/Synthesis/Examples/p224_64_new.v.html │ │ 42.4 41.6 -0.7493 -1.77% 246 coq-fiat-crypto-with-bedrock/src/Bedrock/P256/Coord32.v.html │ │ 65.5 64.8 -0.6869 -1.05% 716 coq-fiat-crypto-with-bedrock/src/Bedrock/Secp256k1/JacobianCoZ.v.html │ │ 18.2 17.6 -0.5770 -3.17% 31 coq-engine-bench-lite/coq/PerformanceDemos/pattern.v.html │ │ 33.3 32.7 -0.5540 -1.67% 12 coq-fourcolor/theories/proof/job439to465.v.html │ │ 45.7 45.2 -0.5427 -1.19% 3 coq-fiat-crypto-with-bedrock/src/ExtractionJsOfOCaml/bedrock2_fiat_crypto.v.html │ │ 33.789 33.278 -0.5110 -1.51% 194 coq-vst/veric/expr_lemmas4.v.html │ │ 31.4 30.9 -0.5030 -1.60% 258 coq-fiat-crypto-with-bedrock/src/Bedrock/P256/Coord.v.html │ │ 26.6 26.2 -0.4529 -1.70% 62 coq-fiat-crypto-with-bedrock/src/Assembly/Parse/TestAsm.v.html │ │ 48.4 48.0 -0.4374 -0.90% 260 coq-fiat-crypto-with-bedrock/src/Bedrock/P256/Jacobian.v.html │ │ 1.44 1.01 -0.4291 -29.81% 690 rocq-stdlib/theories/setoid_ring/Field_theory.v.html │ │ 31.7 31.2 -0.4261 -1.35% 334 coq-fiat-crypto-with-bedrock/src/Bedrock/P256/Coord.v.html │ │ 34.2 33.8 -0.4210 -1.23% 960 coq-fiat-crypto-with-bedrock/src/Bedrock/Secp256k1/JoyeLadder.v.html │ └──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘ |
|
FTR, this kind of change is necessary for fixing #21526. Indeed, to prevent an explosive expansion of the rtree for nested inductive types, one needs a compact representation of rtree nesting. This can only be achieved for inductive types that are uniformly nesting, in which case we can precompute the nesting body as a kind of parameterized rtree. |
Change the computation of the uniform-parameters in the nested case
Fixes / closes #21497
Adapt to rocq-prover/rocq#21498 (changed computation uniform parameters) MetaRocq/metarocq#1233