pke/cryptocontext: expose auto-facade hooks for Make*Plaintext + Eval…#36
Open
podschwadt wants to merge 1 commit into
Open
pke/cryptocontext: expose auto-facade hooks for Make*Plaintext + Eval…#36podschwadt wants to merge 1 commit into
podschwadt wants to merge 1 commit into
Conversation
…*(ct, pt) replay shortcuts Add three pieces of plumbing the niobium-client auto-facade needs to transparently record/replay programs that build plaintexts at runtime (e.g. mask plaintexts for slot-replication, running-sum coefficients): 1. niobium_auto::on_make_plaintext(pt) hook fired at the tail of every MakePackedPlaintext / MakeCKKSPackedPlaintext (both overloads). The weak default is a no-op so existing programs linking only libnbfhetch see no behavioral change. The strong override in libniobium_client_autofacade tags the plaintext as an input under a deterministic auto-name on record, and rehydrates it from disk on replay so the FHETCH simulator has its polynomial values as live-in. In replay mode the hook fires with a null pt (the existing "Make*Plaintext returns nullptr in replay" shortcut is preserved because the encoding work is unnecessary — the recorded values are already on disk). The auto-name counter still advances so record and replay agree on pt_0 / pt_1 / ... ordering. 2. Replay early-returns in the 8 Eval*(ct, pt) wrappers (EvalAdd, EvalAddInPlace, EvalAddMutable, EvalSub, EvalSubInPlace, EvalSubMutable, EvalMult, EvalMultMutable). NiobiumAutoScheme already overrides every one of these with `if (g_replay_mode) return dummy(ct);` — it never dereferences pt. But TypeCheck and plaintext->SetFormat(EVALUATION) in the wrapper run *above* the scheme proxy and would null-deref on the shortcut pt. The early return dispatches straight to GetScheme() in replay so the proxy gets to do its job. Mirrors the EvalAdd(ct, ct) pattern where the proxy naturally short-circuits via the scheme dispatch. 3. !g_replay_mode guard on the evalKeyVec.size() check inside Relinearize / RelinearizeInPlace / EvalMultAndRelinearize. Dummy ciphertexts carry inflated NumberCiphertextElements() that would falsely trip the threshold; the scheme proxy returns dummy(ct) without consulting evalKeyVec anyway. All three pieces are gated on OPENFHE_CPROBES, so non-instrumented builds are unaffected. Programs that build with CPROBES but don't link the auto-facade strong override get weak no-op behavior, identical to prior semantics.
ryanorendorff
left a comment
ryanorendorff
left a comment
There was a problem hiding this comment.
Ideally clean up the // comments because they seem to pull in a bunch of ideas unrelated to the actual PR.
| // from NiobiumAutoScheme carry an inflated NumberCiphertextElements | ||
| // that would falsely trip the threshold. The scheme proxy returns | ||
| // dummy(ct) without ever consulting evalKeyVec. | ||
| if (!g_replay_mode && evalKeyVec.size() < (ciphertext->NumberCiphertextElements() - 2)) |
There was a problem hiding this comment.
There are a few places here that don't have the macro guard (OPENFHE_CPROBES) that I assume brings g_replay_mode into scope; ideally this is consistent so we can disable the probes easily at build time.
Comment on lines
+1210
to
+1214
| if (g_replay_mode) { | ||
| Plaintext null_pt; | ||
| niobium_auto::on_make_plaintext(null_pt); | ||
| return null_pt; | ||
| } |
There was a problem hiding this comment.
Not sure I love this reuse of null as a sentinel value. Also this if statement pattern is repeated, could be factored out.
|
|
||
| const auto& evalKeyVec = CryptoContextImpl<Element>::GetEvalMultKeyVector(ciphertext->GetKeyTag()); | ||
| if (evalKeyVec.size() < (ciphertext->NumberCiphertextElements() - 2)) | ||
| if (!g_replay_mode && evalKeyVec.size() < (ciphertext->NumberCiphertextElements() - 2)) |
There was a problem hiding this comment.
Here as well for missing OPENFHE_CPROBES check.
| const auto& evalKeyVec = CryptoContextImpl<Element>::GetEvalMultKeyVector(ciphertext1->GetKeyTag()); | ||
|
|
||
| if (evalKeyVec.size() < | ||
| if (!g_replay_mode && evalKeyVec.size() < |
There was a problem hiding this comment.
Also since we are skipping checks in a similar manner this should ideally also be abstracted. Otherwise when we look at this code again it will look very odd to skip a necessary check.
| // data — required for replay of any op that consumes the plaintext. | ||
| // In DORMANT mode (explicit niobium::compiler() SDK in use), the | ||
| // override stays a no-op to avoid duplicate-tagging the manual flow. | ||
| void on_make_plaintext(lbcrypto::Plaintext& pt); |
There was a problem hiding this comment.
Ideally we don't have a symbol we require a downstream TU to provide to link openfhe. You could add a no-op default here.
I am also unsure what the definition of this is supposed to be.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…*(ct, pt) replay shortcuts
Add three pieces of plumbing the niobium-client auto-facade needs to transparently record/replay programs that build plaintexts at runtime (e.g. mask plaintexts for slot-replication, running-sum coefficients):
niobium_auto::on_make_plaintext(pt) hook fired at the tail of every MakePackedPlaintext / MakeCKKSPackedPlaintext (both overloads). The weak default is a no-op so existing programs linking only libnbfhetch see no behavioral change. The strong override in libniobium_client_autofacade tags the plaintext as an input under a deterministic auto-name on record, and rehydrates it from disk on replay so the FHETCH simulator has its polynomial values as live-in.
In replay mode the hook fires with a null pt (the existing "Make*Plaintext returns nullptr in replay" shortcut is preserved because the encoding work is unnecessary — the recorded values are already on disk). The auto-name counter still advances so record and replay agree on pt_0 / pt_1 / ... ordering.
Replay early-returns in the 8 Eval*(ct, pt) wrappers (EvalAdd, EvalAddInPlace, EvalAddMutable, EvalSub, EvalSubInPlace, EvalSubMutable, EvalMult, EvalMultMutable). NiobiumAutoScheme already overrides every one of these with
if (g_replay_mode) return dummy(ct);— it never dereferences pt. But TypeCheck and plaintext->SetFormat(EVALUATION) in the wrapper run above the scheme proxy and would null-deref on the shortcut pt. The early return dispatches straight to GetScheme() in replay so the proxy gets to do its job. Mirrors the EvalAdd(ct, ct) pattern where the proxy naturally short-circuits via the scheme dispatch.!g_replay_mode guard on the evalKeyVec.size() check inside Relinearize / RelinearizeInPlace / EvalMultAndRelinearize. Dummy ciphertexts carry inflated NumberCiphertextElements() that would falsely trip the threshold; the scheme proxy returns dummy(ct) without consulting evalKeyVec anyway.
All three pieces are gated on OPENFHE_CPROBES, so non-instrumented builds are unaffected. Programs that build with CPROBES but don't link the auto-facade strong override get weak no-op behavior, identical to prior semantics.