Revert change and clean datatypes cons candidate generator (#7645)
authorAndrew Reynolds <andrew.j.reynolds@gmail.com>
Wed, 17 Nov 2021 12:29:14 +0000 (06:29 -0600)
committerGitHub <noreply@github.com>
Wed, 17 Nov 2021 12:29:14 +0000 (12:29 +0000)
PR #7600 refactored and optimized candidate generator. This PR actually corrected an issue where the datatypes constructor expansion candidate generator did not generate instances for triggers whose top symbol is a constructor. However, fixing it to generate instances led to a performance regression on Facebook benchmarks.

This PR reverts the behavior change from that PR, and moreover makes it explicit that we do not generate instances in this case.

src/options/quantifiers_options.toml
src/theory/quantifiers/ematching/candidate_generator.cpp
src/theory/quantifiers/ematching/candidate_generator.h

index e53d3f5ba291e419391824c79735f25da8b60f14..80692055f8e9ad3f2b2e7d50b6484d4e13e176d2 100644 (file)
@@ -258,6 +258,14 @@ name   = "Quantifiers"
   default    = "false"
   help       = "use triggers that do not contain all free variables"
 
+[[option]]
+  name       = "consExpandTriggers"
+  category   = "regular"
+  long       = "cons-exp-triggers"
+  type       = "bool"
+  default    = "false"
+  help       = "use constructor expansion for single constructor datatypes triggers"
+
 [[option]]
   name       = "multiTriggerWhenSingle"
   category   = "regular"
index 513a3965b199c8f208f5d0b385b3d47baad114f2..57752d3752d6db8544a22980cbababacbbe3eaaa 100644 (file)
@@ -64,11 +64,7 @@ void CandidateGeneratorQE::resetForOperator(Node eqc, Node op)
   d_eqc = eqc;
   d_op = op;
   d_termIterList = d_treg.getTermDatabase()->getGroundTermList(d_op);
-  if (d_termIterList == nullptr)
-  {
-    d_mode = cand_term_none;
-  }
-  else if (eqc.isNull())
+  if (eqc.isNull())
   {
     d_mode = cand_term_db;
   }else{
@@ -109,11 +105,15 @@ Node CandidateGeneratorQE::getNextCandidate(){
 Node CandidateGeneratorQE::getNextCandidateInternal()
 {
   if( d_mode==cand_term_db ){
-    Assert(d_termIterList != nullptr);
+    if (d_termIterList == nullptr)
+    {
+      d_mode = cand_term_none;
+      return Node::null();
+    }
     Debug("cand-gen-qe") << "...get next candidate in tbd" << std::endl;
     //get next candidate term in the uf term database
-    size_t tlSize = d_termIterList->d_list.size();
-    while (d_termIter < tlSize)
+    size_t tlLimit = d_termIterList->d_list.size();
+    while (d_termIter < tlLimit)
     {
       Node n = d_termIterList->d_list[d_termIter];
       d_termIter++;
@@ -254,8 +254,17 @@ void CandidateGeneratorConsExpand::reset(Node eqc)
   d_termIter = 0;
   if (eqc.isNull())
   {
-    d_termIterList = d_treg.getTermDatabase()->getGroundTermList(d_op);
-    d_mode = d_termIterList == nullptr ? cand_term_none : cand_term_db;
+    // generates too many instantiations at top-level when eqc is null, thus
+    // set mode to none unless option is set.
+    if (options::consExpandTriggers())
+    {
+      d_termIterList = d_treg.getTermDatabase()->getGroundTermList(d_op);
+      d_mode = cand_term_db;
+    }
+    else
+    {
+      d_mode = cand_term_none;
+    }
   }
   else
   {
index 5c85d118fd2d5e368a874e187cbe79846d6756fc..3b4017e999eda733b5063e2720bb8d95ebe6144c 100644 (file)
@@ -206,7 +206,7 @@ class CandidateGeneratorQEAll : public CandidateGenerator
  * This modifies the candidates t1, ..., tn generated by CandidateGeneratorQE
  * so that they are "expansions" of a fixed datatype constructor C. Assuming
  * C has arity m, we instead return the stream:
- *   C(sel_1( t1 ), ..., sel_m( tn )) ... C(sel_1( t1 ), ..., C( sel_m( tn ))
+ *   C(sel_1( t1 ), ..., sel_m( t1 )) ... C(sel_1( tn ), ..., C( sel_m( tn ))
  * where sel_1 ... sel_m are the selectors of C.
  */
 class CandidateGeneratorConsExpand : public CandidateGeneratorQE