Bugfix: test for m being a power of two was wrong

Author: shaih

src/FHEContext.cpp

@@ -195,9 +195,9 @@ long FHEcontext::AddFFTPrime(bool special)
   return p;
 }
 
-// Adds several primes to the chain. If byNumber=true then totalSize
-// specifies the number of primes to add. If byNumber=false then
-// totalSize specifies the target natural log all the added primes.
+// Adds several primes to the chain. If byNumber=true then totalSize specifies
+// the number of primes to add. If byNumber=false then totalSize specifies the
+// target natural log all the added primes.
 // Returns natural log of the product of all added primes.
 double AddManyPrimes(FHEcontext& context, double totalSize, 
 		     bool byNumber, bool special)
@@ -206,10 +206,6 @@ double AddManyPrimes(FHEcontext& context, double totalSize,
     Error("AddManyPrimes: m undefined or larger than 2^20");
   // NOTE: Below we are ensured that 16m*log(m) << NTL_SP_BOUND
 
-  // cout << "AddManyPrimes(..., totalSize="<<((int)totalSize)
-  //      << ",byNumber="<<byNumber<<",special="<<special<<")\n";
-  // cout << "  context.bitsPerLevel="<<context.bitsPerLevel<<endl;
-
   double sizeLogSoFar = 0.0; // log of added primes so far
   double addedSoFar = 0.0;   // Either size or number, depending on 'byNumber'
 
@@ -219,9 +215,9 @@ double AddManyPrimes(FHEcontext& context, double totalSize,
   long sizeBits = 2*context.bitsPerLevel;
 #endif
   if (special) { // try to use similar size for all the special primes
-    // how many special primes would we need
-    long numPrimes = ceil(totalSize/(NTL_SP_NBITS*log(2.0)));
-    sizeBits = 1+ceil(totalSize/(log(2.0)*numPrimes)); // bitsize of each prime
+    double totalBits = totalSize/log(2.0);
+    long numPrimes = ceil(totalBits/NTL_SP_NBITS);// how many special primes
+    sizeBits = 1+ceil(totalBits/numPrimes);       // what's the size of each
     // Added one so we don't undershoot our target
   }
   if (sizeBits>NTL_SP_NBITS) sizeBits = NTL_SP_NBITS;
@@ -235,7 +231,9 @@ double AddManyPrimes(FHEcontext& context, double totalSize,
   }
 
   // make p-1 divisible by m*2^k for as large k as possible
-  if (context.zMStar.getPow2()!=0) // if m is not a power of two
+  // (not needed when m itself a power of two)
+
+  if (context.zMStar.getM() & 1) // m is odd, so not power of two
     while (twoM < sizeBound/(sizeBits*2)) twoM *= 2;
 
   long bigP = sizeBound - (sizeBound%twoM) +1; // 1 mod 2m
@@ -256,10 +254,8 @@ double AddManyPrimes(FHEcontext& context, double totalSize,
   return sizeLogSoFar;
 }
 
-void buildModChain(FHEcontext &context, long nLevels, long nDgts)
+void buildModChain(FHEcontext &context, long nLevels, long nDgts,long extraBits)
 {
-  //  cout << "buildModChain called with "<<nLevels
-  //       <<" levels and "<<nDgts<<" digits\n";
 #ifdef NO_HALF_SIZE_PRIME
   long nPrimes = nLevels;
 #else
@@ -290,7 +286,7 @@ void buildModChain(FHEcontext &context, long nLevels, long nDgts)
   context.digits.resize(nDgts); // allocate space
 
   IndexSet s1;
-  double sizeLogSoFar = 0.0;
+  double sizeSoFar = 0.0;
   double maxDigitSize = 0.0;
   if (nDgts>1) { // we break ciphetext into a few digits when key-switching
     double dsize = context.logOfProduct(context.ctxtPrimes)/nDgts; // estimate
@@ -301,10 +297,9 @@ void buildModChain(FHEcontext &context, long nLevels, long nDgts)
     long idx = context.ctxtPrimes.first();
     for (long i=0; i<nDgts-1; i++) { // set all digits but the last
       IndexSet s;
-      while (idx <= context.ctxtPrimes.last()
-             && (empty(s)||sizeLogSoFar<target)) {
+      while (idx <= context.ctxtPrimes.last() && (empty(s)||sizeSoFar<target)) {
         s.insert(idx);
-	sizeLogSoFar += log((double)context.ithPrime(idx));
+	sizeSoFar += log((double)context.ithPrime(idx));
 	idx = context.ctxtPrimes.next(idx);
       }
       assert (!empty(s));
@@ -332,11 +327,10 @@ void buildModChain(FHEcontext &context, long nLevels, long nDgts)
   }
 
   // Add special primes to the chain for the P factor of key-switching
-  long p2r = (context.rcData.alMod)? context.rcData.alMod->getPPowR()
-                                   : context.alMod.getPPowR();
+  long p2r = context.alMod.getPPowR();
   double sizeOfSpecialPrimes
     = maxDigitSize + log(nDgts) + log(context.stdev *2)
-      + log((double)p2r);
+      + log((double)p2r) + (extraBits*log(2.0));
 
   AddPrimesBySize(context, sizeOfSpecialPrimes, true);
 }

src/FHEContext.h

@@ -289,7 +289,11 @@ inline double AddPrimesByNumber(FHEcontext& context, long nPrimes,
 }
 
 //! @brief Build modulus chain with nLevels levels, using c digits in key-switching
-void buildModChain(FHEcontext &context, long nLevels, long c=3);
+void buildModChain(FHEcontext &context, long nLevels, long c=3,
+                   long extraBits=0);
+//FIXME: The extraBits params is a hack, used to get around some
+//       circularity when making the context boostrappable
+
 ///@}
 extern FHEcontext* activeContext; // Should point to the "current" context
 #endif // ifndef _FHEcontext_H_

src/Test_bootstrapping.cpp

@@ -92,14 +92,14 @@ static long mValues[][14] = {
   {127, 54400, 61787, 40, 41, 1507, 0, 30141, 46782,    0, 40, 34,   0, 100}, // m=(11)*41*{137} m/phim(m)=1.13  C=112 D=2
   {127, 72000, 77531, 30, 61, 1271, 0,  7627, 34344,    0, 60, 40,   0, 100}  // m=(31)*{41}*61 m/phim(m)=1.07   C=128 D=2
 };
-#define num_mValues (sizeof(mValues)/(13*sizeof(long)))
+#define num_mValues (sizeof(mValues)/(14*sizeof(long)))
 
 #define OUTER_REP (3)
 #define INNER_REP (3)
 
 static bool dry = false; // a dry-run flag
 
-void TestIt(long idx, long p, long r, long L, long c, long B, long skHwt, bool cons=false, int cType=0)
+void TestIt(long idx, long p, long r, long L, long c, long B, long skHwt, bool cons=false, int cacheType=0)
 {
   Vec<long> mvec;
   vector<long> gens;
@@ -138,8 +138,14 @@ void TestIt(long idx, long p, long r, long L, long c, long B, long skHwt, bool c
   double t = -GetTime();
   FHEcontext context(m, p, r, gens, ords);
   context.bitsPerLevel = B;
-  buildModChain(context, L, c);
-  context.makeBootstrappable(mvec, /*t=*/0, cons, cType);
+  buildModChain(context, L, c,/*extraBits=*/7);
+
+  // FIXME: The extraBits is an exceedingly ugly patch, used to bypass the
+  //   issue that buildModChain must be called BEFORE the context is made
+  //   bootstrappable (else the "powerful" basis is not initialized correctly.)
+  //   This is a bug, the value 7 is sometimes the right one, but seriously??
+
+  context.makeBootstrappable(mvec, /*t=*/0, cons, cacheType);
   t += GetTime();
 
   if (skHwt>0) context.rcData.skHwt = skHwt;