ssr: skip impargs in pattern FO unif#20707
ssr: skip impargs in pattern FO unif#20707coqbot-app[bot] merged 3 commits intorocq-prover:masterfrom
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
Great!!! There will be legitimate breakage! Maybe we need a flag ? |
This comment was marked as outdated.
This comment was marked as outdated.
|
FWIW, a quick look at mathcomp show 85 proofs requiring an update. |
016614c to
5559aa7
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
On the positive side, most of the broken proof were bad ones, i.e. relying in some way on the FO pass to select "the wrong one". The only cases where now things are reversed are because of the behavior above, IIRC 2 occurrences in MC. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
5559aa7 to
6a10c2d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
6a10c2d to
cf56c25
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
55afb08 to
7371663
Compare
|
Thanks @tchajed here is the (trivial and backward compatible) overlay: mit-pdos/perennial#279 |
|
@proux01 I plan to test this PR on the wrapping branch with Matteo on Tuesday. Apparently it is not solving all problems as we expected, it may need more fine tuning. |
|
Yes, there still seems to be some problems with the unification of the notation "^+" which is present both on the ring and fingroup scope, This is the error i get (for instance in Despite this is under The workaround is to annote the context as in 'Check _ * (y^+n)%R.' |
Adapt to rocq-prover/rocq#20707 This should obviously be backward compatible.
|
Well, this is ready with (almost) all overlays merged, so I'd be inclined to merge it and let any further improvement happen in a new PR. Anyway, this won't land in 9.1 so changes won't be spread across multiple releases. Let's do the following:
|
|
@coqbot bench |
Not sure I understand the link between
Again, this is just changing the notation interpretation, hard to see any clear link with pattern selection. |
Totally agree, I just want to look at the rewrite errors. This PR and overlays are sound I believe. If we identify a needed change that requires no overlay update we will propose it, otherwise we make a separate pr. |
|
🏁 Bench results: INFO: failed to install 🐢 Top 25 slow downs┌──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐ │ TOP 25 SLOW DOWNS │ │ │ │ OLD NEW DIFF %DIFF Ln FILE │ ├──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤ │ 0.205 3.55 3.3458 1633.37% 403 coq-mathcomp-odd-order/theories/BGappendixC.v.html │ │ 0.0461 1.78 1.7343 3762.00% 394 coq-mathcomp-odd-order/theories/BGappendixC.v.html │ │ 0.0414 1.75 1.7086 4130.00% 645 coq-mathcomp-analysis/theories/tvs.v.html │ │ 38.8 40.1 1.3233 3.41% 236 coq-rewriter/src/Rewriter/Rewriter/Examples/PerfTesting/LiftLetsMap.v.html │ │ 0.784 1.83 1.0461 133.48% 395 coq-mathcomp-odd-order/theories/BGappendixC.v.html │ │ 1.07 1.50 0.4300 40.02% 75 rocq-stdlib/theories/Numbers/HexadecimalString.v.html │ │ 11.7 12.1 0.3425 2.92% 125 coq-mathcomp-odd-order/theories/BGappendixAB.v.html │ │ 24.0 24.2 0.2745 1.15% 558 coq-bedrock2/bedrock2/src/bedrock2Examples/insertionsort.v.html │ │ 25.9 26.2 0.2700 1.04% 12 coq-fourcolor/theories/proof/job190to206.v.html │ │ 18.7 19.0 0.2678 1.43% 481 coq-verdi-raft/theories/RaftProofs/EndToEndLinearizability.v.html │ │ 28.0 28.2 0.2617 0.94% 12 coq-fourcolor/theories/proof/job563to588.v.html │ │ 1.39 1.65 0.2576 18.53% 313 rocq-stdlib/theories/Strings/Byte.v.html │ │ 17.9 18.1 0.2456 1.38% 32 coq-performance-tests-lite/src/pattern.v.html │ │ 0.307 0.524 0.2168 70.56% 586 rocq-stdlib/theories/Strings/Byte.v.html │ │ 25.8 26.1 0.2088 0.81% 12 coq-fourcolor/theories/proof/job399to438.v.html │ │ 0.000240 0.204 0.2041 85025.00% 429 rocq-metarocq-safechecker/safechecker/theories/PCUICSafeChecker.v.html │ │ 0.0106 0.215 0.2039 1921.02% 766 rocq-metarocq-erasure/erasure/theories/EWcbvEval.v.html │ │ 0.0391 0.241 0.2017 515.94% 1804 rocq-stdlib/theories/MSets/MSetRBT.v.html │ │ 0.349 0.550 0.2011 57.60% 14 rocq-stdlib/theories/extraction/ExtrOcamlZInt.v.html │ │ 0.0116 0.212 0.2005 1733.96% 143 rocq-mathcomp-field/field/algebraics_fundamentals.v.html │ │ 0.458 0.657 0.1988 43.38% 10 rocq-mathcomp-character/character/vcharacter.v.html │ │ 2.27 2.47 0.1953 8.60% 607 rocq-stdlib/theories/Zmod/ZmodBase.v.html │ │ 0.0112 0.198 0.1872 1675.40% 359 rocq-metarocq-pcuic/pcuic/theories/PCUICCasesHelper.v.html │ │ 0.0250 0.212 0.1866 747.57% 432 rocq-metarocq-pcuic/pcuic/theories/PCUICInductiveInversion.v.html │ │ 0.00958 0.195 0.1853 1935.22% 561 rocq-metarocq-pcuic/pcuic/theories/PCUICSR.v.html │ └──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘ 🐇 Top 25 speed ups┌─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐ │ TOP 25 SPEED UPS │ │ │ │ OLD NEW DIFF %DIFF Ln FILE │ ├─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤ │ 8.56 0.0160 -8.5478 -99.81% 925 coq-mathcomp-odd-order/theories/PFsection4.v.html │ │ 8.42 0.00606 -8.4182 -99.93% 199 coq-mathcomp-odd-order/theories/PFsection12.v.html │ │ 66.0 62.6 -3.4312 -5.20% 609 coq-bedrock2/bedrock2/src/bedrock2Examples/lightbulb.v.html │ │ 3.68 2.44 -1.2365 -33.61% 1198 coq-mathcomp-odd-order/theories/PFsection9.v.html │ │ 3.58 2.38 -1.1994 -33.47% 986 coq-mathcomp-odd-order/theories/PFsection9.v.html │ │ 93.9 93.0 -0.8673 -0.92% 968 coq-performance-tests-lite/src/fiat_crypto_via_setoid_rewrite_standalone.v.html │ │ 93.7 92.9 -0.8168 -0.87% 999 coq-performance-tests-lite/src/fiat_crypto_via_setoid_rewrite_standalone.v.html │ │ 199 198 -0.5727 -0.29% 8 coq-neural-net-interp-computed-lite/theories/MaxOfTwoNumbersSimpler/Computed/AllLogits.v.html │ │ 0.450 0.0264 -0.4236 -94.13% 1905 coq-mathcomp-odd-order/theories/PFsection13.v.html │ │ 3.96 3.56 -0.4001 -10.11% 492 rocq-stdlib/theories/Reals/Cauchy/ConstructiveCauchyRealsMult.v.html │ │ 1.67 1.27 -0.3989 -23.85% 1207 rocq-metarocq-erasure/erasure/theories/ErasureFunction.v.html │ │ 1.11 0.782 -0.3276 -29.52% 1408 coq-mathcomp-odd-order/theories/PFsection9.v.html │ │ 1.29 0.988 -0.3008 -23.34% 572 rocq-stdlib/theories/MSets/MSetAVL.v.html │ │ 31.721 31.425 -0.2960 -0.93% 97 coq-vst/veric/binop_lemmas5.v.html │ │ 33.314 33.039 -0.2750 -0.83% 194 coq-vst/veric/expr_lemmas4.v.html │ │ 34.379 34.109 -0.2700 -0.79% 147 coq-vst/veric/expr_lemmas4.v.html │ │ 40.862 40.616 -0.2460 -0.60% 834 coq-vst/veric/binop_lemmas4.v.html │ │ 1.64 1.42 -0.2246 -13.70% 73 rocq-stdlib/theories/Numbers/HexadecimalString.v.html │ │ 9.66 9.44 -0.2210 -2.29% 87 coq-engine-bench-lite/coq/PerformanceDemos/quadratic_reduction.v.html │ │ 36.6 36.4 -0.2191 -0.60% 139 coq-fiat-parsers/src/Parsers/Refinement/SharpenedJSON.v.html │ │ 2.57 2.36 -0.2075 -8.07% 447 coq-unimath/UniMath/CategoryTheory/DisplayedCats/Examples/StrictTwoSidedDispCat.v.html │ │ 0.208 0.00210 -0.2058 -98.99% 428 rocq-metarocq-safechecker/safechecker/theories/PCUICSafeChecker.v.html │ │ 0.205 0.000356 -0.2047 -99.83% 765 rocq-metarocq-erasure/erasure/theories/EWcbvEval.v.html │ │ 5.08 4.88 -0.1935 -3.81% 121 coq-unimath/UniMath/CategoryTheory/Hyperdoctrines/PartialEqRels/ExponentialEqs.v.html │ │ 0.243 0.0539 -0.1888 -77.79% 429 rocq-metarocq-pcuic/pcuic/theories/PCUICInductiveInversion.v.html │ └─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘ |
|
OK, we confirm the PR fixes all the matching related problems we have in the wrapping branch. Green light. |
|
So, only waiting for the coquelicot overlay to be merged and I merge. |
|
@coqbot merge now |
|
@proux01: You can't merge the PR because it hasn't been approved yet. |
|
@coqbot merge now |
|
@proux01: Please take care of the following overlays:
|
Adapt to rocq-prover/rocq#20707 This should obviously be backward compatible.
adapt to rocq-prover/rocq#20707 See merge request iris/iris!1124
Adapt to rocq-prover/rocq#20707 See merge request coquelicot/coquelicot!14
This is an attempt to make changes in the algebraic hierarchy of mathcomp less painful.
When a structure s with op1 and op2 is broken into s1 with op1 and s2 with op2 it may be the case that lemmas involving both operations have a type like
forall i:sj, op1 i x (op2 i y) z = ..where an instance of the structure sj (join of s1 and s2) occurs non linearly. A concrete goal may have after the split the shapeop1 canon_s1_on_T x (op2 (sj_to_s2 canon_sj_on_T) y) zor something like that, while before both ops would have the same canonical instance on T (the type of x, y, z).Because of the 2 syntactically different instances the FO pass makes a different choice than before.
This PR tries to avoid breakage by making the FO pass do
ignore implicit arguments of constants, e.g. structure instances
in one case the real arg is implicit because it is followed by a phantom, hence I use the shortest list of impargsNot needed/used anymore, but still valid: rewrite takes the shortest list of impargs among the existing ones.
ignore subterms that are FO-different but the pattern is is a proj
reduce the goal subterm if it is not equal to the pattern and is a proj (applied to a CS instance)
fail to match if the pattern contains more evars that the matched goal (i.e. if it contains non-instantiated variables)
flags
Set SsrMatching LegacyFoUnifmakes it behave as before, so one can compare old/new using the same version of Rocq and/or give up fixing a proof.Things tried and abandoned
completely disable FO pass
_ (if ..)or_ f grely on the FO pass. I tried adjusting the HO one to behave the same (use the first arg key rigidly) but some lemmas nee more keys...do not ignore
proj _ = tproblems, since in groups we have plenty of set/group lemmas, with gval that would behave surprisingly, egx \in <[x]> || x \in GwhereG : group, the second one has an explicit gval, the former none, the lemma has it.Added / updated test-suite.
I don't think the FO pass was ever documented, and I'm not so sure it should. This PR contains the details for the braves.
make doc_gram_rsts.