Support per-dimension gadget bases in FHEW bootstrapping - #1268
Conversation
Review summaryThe core mechanism looks correct — per-coefficient gadget bases are propagated consistently between key generation and accumulator evaluation in all three blind-rotation methods, and all pre-existing parameter sets remain behaviorally identical (confirmed by green CI across all backends). However, there is one issue that should block merge as-is (serialization compatibility), plus a test-coverage gap and a few latent hazards worth addressing. Blocking issueSerialization break without a version bump. Suggested fix: bump Note that even with that fix, Verified as correct
Should-fix items
Minor notes
|
6327ebf to
d8c5aa7
Compare
d8c5aa7 to
f24598a
Compare
ceil(log_base(x)) was computed as std::ceil(std::log(x) / std::log(base)) in the gadget-decomposition, key-switching and trapdoor paths. The result depends on libm rounding and on converting the modulus to a double, which loses bits above 2^53, so the digit count could come out one too high -- wasting key material and work -- or one too low, leaving a gadget that does not span the modulus. Across bases 2 to 300 the two disagree on 1498 of 14087 boundary neighbourhoods, in both directions. Add GetDigitCount() to nbtheory.h: a shift for power-of-two bases and a division loop otherwise, exact for any base and any modulus width. Use it for the RingGSW gadget bases and the AP refreshing base, for the LWE key-switching digit count -- now derived in LWECryptoParams so that key generation and key switching cannot drift apart -- and for the DCRTPoly trapdoor and gadget vector.
Both SignedDigitDecompose implementations bias the centred coefficient by H = (baseG/2) * (baseG^digitsG - 1)/(baseG - 1) and then read digitsG masked windows. H is strictly greater than baseG^digitsG / 2, so Q/2 + H can exceed baseG^digitsG, and masking the top window discards that carry: the decomposition reconstructs the coefficient short by baseG^digitsG mod Q. digitsG = ceil(log_baseG(Q)) does not prevent this, and the only precondition noted in the code is the unrelated word-width one. Ten of the fifty shipped (parameter set, gadget base) pairs are affected in the ciphertext overload, between 0.01% and 1.6% of coefficients each: TOY, TOY_MULTI_BASE, STD128_AP and STD128_LMKCDEY at base 512, STD192Q_LMKCDEY at base 4096, and SIGNED_MOD_TEST at base 128, whose residual reached 65598 against an allowed 64. The single-NativePoly overload, used by the LMKCDEY automorphism path, decomposes with the default base and is affected for STD128_LMKCDEY and STD192Q_LMKCDEY. This is a regression from #1238, which replaced the carry-propagating decomposition with the excess-H form. Leave the top window unmasked in both so it absorbs the carry. The biased value stays below 2*baseG^digitsG, so that digit is bounded by 2*baseG: no extra digit, no change to key sizes or to the work per gate. The mask is selected once per digit rather than per coefficient, and the gate suite measures unchanged (geomean 0.998 single-threaded, 1.000 at 36 threads). Assign the digits instead of accumulating them. Every call site passes a buffer that is written exactly once, so the read-modify-write was redundant and the routine silently depended on the caller having pre-zeroed it. Every shipped pair now reconstructs with a residual of exactly baseG/2, the bound the approximate decomposition allows. In a configuration with every coefficient at an affected base, the fix is worth 74 bits of decryption failure probability.
Change_BaseG() updates m_baseG, m_digitsG and m_Gpower, but not the per-index table that PreCompute() builds from the gadget base map. The accumulator reads through GetBaseGParams(index), which consults those default members only when the table is empty -- so on a context constructed with a base map, Change_BaseG mutated state nothing reads: it appeared to succeed and changed nothing about the decomposition. The two features cannot currently meet. Change_BaseG serves the runtime base switching in EvalSign/EvalDecomp and the per-base key pregeneration in BTKeyGen, both of which need m_Gpower_map populated with 2^14/2^18/2^27 -- and that only happens under PreCompute(signEval=true), which is set at exactly one call site, the arbFunc constructor, which uses the single-base constructor. Every map-carrying path passes signEval=false. Nothing stated or enforced this, so a user calling GenerateBinFHEContext(BinFHEContextParams) with a gadgetBaseMap and then EvalFloor/EvalSign would have walked into it. We have made Change_BaseG throw when a base map is present, so the incompatibility surfaces at the point of misuse instead of silently doing nothing. This has no effect on any path that works today. The alternative reading is that Change_BaseG should rebuild the per-index table uniformly with the new base -- i.e. an override of all per-coefficient assignments -- which is coherent but needs gadget powers for the new base, and it is not obvious that a global override is meaningful once per-coefficient bases have been chosen deliberately. We picked the conservative option; happy to switch to the override semantics if that was the intent.
|
The capability is sound and we'll add it now, but the four LPF configurations shipped with this PR The configurations don't make use of the capabilityicelake, clang18, min of 3 reps, this PR vs its parent. Geomean over 13 gate/keygen rows
The speedup tracks the fraction of coefficients moved to the larger base, which is the Worth adding for #1285: the LWE key switching is 88% of total noise variance in CleanupI've also made a number of formatting, cleanup and optimization changes on top — the Open question:
|
BinFHEContext::BTKeyGen caches bootstrapping keys in m_BTKey_map, which is keyed by gadget base alone -- but what it caches is only valid for the secret key it was generated from, and nothing enforced that. Without the time optimization, the else-branch populated the map, so the next call took the cache-hit branch and assigned m_BTKey from it, ignoring the sk it was passed. A second BTKeyGen(sk2) on the same context therefore returned sk1's key silently -- no exception, no warning -- and every subsequent decryption returned noise. Also compute gadget powers on demand in Change_BaseG. It looked the base up in m_Gpower_map and threw when absent, but a deserialized context reaches that state legitimately: RingGSWCryptoParams::load() calls PreCompute() with signEval defaulted to false, so it lacks the sign-evaluation bases the generating context had, and both boolean-serial-*-dynamic-large-precision examples aborted on Change_BaseG(1 << 27). Filling the entry instead fixes that without restoring the previous behaviour, where operator[] inserted an empty vector for the accumulator to read past the end of. Add regression tests for two secret keys on one context, with and without the time optimization. The first fails before this change. The second is excluded at NATIVE_SIZE=32 as in UnitTestFunc.cpp, because the large-precision constructor sets qKS = 1 << 35, which truncates to zero in a 32-bit NativeInteger.
|
Following up on the How we got there: we ran into this while updating our lattice-estimator parameter The deciding factor was Throwing also keeps the door open for what we suspect is the real answer if these two |
Below is a brief summary of the main changes.
The existing parameter structure, which previously supported a single
baseG, has been extended to support multiple gadget bases. AgadgetBaseMapand the corresponding internalm_baseG_mapwere added, while the existingm_Gpower_mapis used to store the precomputed gadget powers for each base.GetBaseG()andGetDigitsG()now take an LWE secret-key coefficient index and return the gadget base and the corresponding number of decomposition digits assigned to that coefficient. To support this throughout the bootstrapping path, the LWE secret-key index is propagated through key generation and accumulator updates so that the appropriate gadget base can be selected duringEvalACC. A new constructor that takes agadgetBaseMapwas also added.The named parameter sets are now initialized with their corresponding gadget base maps. In addition, the parameter sets with the$2^{-128}$ .
LPF_prefix were updated using theopenfhe-lattice-estimatorso that their failure probability is approximatelyThe same estimator can also be used to search for parameter sets targeting an arbitrary failure probability. The updated
LPF_parameter sets in this PR were obtained using this parameter search.