feat: allow custom lookup paths for nonlinear ops and fix Python GIL deadlock#1023
feat: allow custom lookup paths for nonlinear ops and fix Python GIL deadlock#1023changshenhan wants to merge 9 commits into
Conversation
…ase GIL during prove
|
Hi @jasonmorton @alexander-camuto @JSeam2,Sorry for the ping, but I wanted to share some compelling end-to-end benchmark results I just finished, which might be helpful for the review of this PR.In a full Conv+ReLU+Sigmoid forward pass, using the custom 1024-segment PWL proposed in this PR:Precision: Reduced the Mean Absolute Error (MAE) from |
|
I have triggered some initial tests, this looks interesting. For this to be mergeable we need a few more things
Some caveat about usability
|
… generate_pwl_json and 1e-11 note
… no personal link
Thanks for the encouraging feedback, I really appreciate the guidance on making this more robust for the community. I've addressed your points with a focus on usability, soundness, and seamless integration: 1. Usability & documentation
2. Soundness & error handling
3. Testing & integration
I've included Please let me know if there are any other areas to refine—looking forward to your thoughts! |
Move secret usage into dedicated GitHub Environments and replace runner-superfluous actions with native script steps, so `zizmor .` exits cleanly without relying on a repo-level suppression config. Made-with: Cursor
|
Hi @JSeam2, I've successfully addressed all the static analysis findings reported by zizmor. What’s been updated: Hardened Security: Added environment declarations for jobs accessing secrets to resolve secrets-outside-env warnings. Refactored Workflows: Replaced several redundant third-party actions with native GitHub Runner commands (e.g., using gh CLI and rustup) to fix superfluous-actions and reduce supply chain surface. Cleaned up: Removed the zizmor.yml override since the workflows are now natively compliant with the audit. The CI for Static Analysis is now passing with a clean exit code. Ready for your further review! |
Add optional custom lookup table for Sigmoid (PWL from JSON)
Problem
EZKL implements Sigmoid (and other nonlinearities) via fixed built‑in lookup tables. There is currently no way for users to:
This makes it harder to:
Approach
Add an optional run arg:
When
custom_lookup_pathis set:Every ONNX Sigmoid node is implemented as
instead of
The file at
pathis a JSON with{ "breakpoints": [...], // length n+1 "slopes": [...], // length n "intercepts": [...] // length n }defining a piecewise‑linear map. The lookup table is filled by evaluating this PWL over the configured
lookup_range.The same Halo2 lookup constraint machinery as for
LookupOp::Sigmoidis used; only the table contents are user‑defined.When
custom_lookup_pathisNoneor unset, behavior is unchanged from currentmain(fully backward compatible).Design choices
lazy_static+Mutex) for loaded PWLproveprove()calls into halo2, which uses rayon for FFT/commit. If the main thread holds the GIL while blocking on rayon, deadlocks can occur. Wrapping the prove call inpy.allow_threads(...)releases the GIL so that workers can run; this was necessary when using the custom lookup path from Python.Compatibility with existing
LookupOpConstraint system: No structural change.
Customuses the same table layout and lookup argument asLookupOp::Sigmoid; only the way table cells are filled differs (user PWL vs. built‑in σ(x)).Serialization:
LookupOpgains a new variantExisting configs that do not reference
Customare unaffected.RunArgs:
custom_lookup_pathis anOption<String>with#[serde(default)], so existing JSON configs and CLI usage remain valid.Files touched
src/lib.rs: addcustom_lookup_pathtoRunArgs, defaulting toNone.src/bindings/python.rs:custom_lookup_pathtoPyRunArgs, and pass it through in conversions;prove()inpy.allow_threads(...)to release the GIL while halo2/rayon runs.src/circuit/ops/lookup.rs:LookupOp::Custom { scale, path };PwlParams(breakpoints/slopes/intercepts), JSON load + global cache;f()forCustomby evaluating the PWL map over the integer‑scaled input;log::warn!for soundness.src/graph/utilities.rs: in the ONNX"Sigmoid"branch, ifrun_args.custom_lookup_pathisSome(path), emitLookupOp::Custom { scale, path }, otherwise keep the existingLookupOp::Sigmoid { scale }.src/circuit/table.rs: rely onnonlinearity.f()to produce table contents; forCustomthis means evaluating the user PWL. Optional logs for custom lookup.src/pfsys/mod.rs: add an optional heartbeat duringcreate_proofand fix anmpsc::channel::<()>()type issue.examples/notebooks/custom_lookup_demo.ipynb: new demo notebook withgenerate_pwl_json(uniform / curvature / quantile / custom), full pipeline, production tip.examples/pwl_sigmoid_example.json: example PWL file.docs/custom_lookup_table.md: JSON schema, step‑by‑step guide, caveats (including input range and margin).README.md: short “Custom lookup table” line and link to the doc.tests/py_integration_tests.rs: registercustom_lookup_demo.ipynb.tests/integration_tests.rs: addmock_custom_lookup_1l_sigmoidand optionalcustom_lookup_pathingen_circuit_settings_and_witness.Accuracy and performance (clarified)
To avoid confusion: the main benefit of this PR is numerical accuracy and flexibility, not a dramatic asymptotic speedup.
For a small Conv+ReLU+Sigmoid toy model with
num_rows ≈ 4,385:logrows = 17, the lookup circuit pays a larger SRS and longer proving time.logrows = ceil(log2(num_rows)) = 13, both the built‑in Sigmoid lookup and the custom PWL lookup prove in about 1.1 s on the same machine.Single‑Sigmoid input level (real input distribution, same circuit/logrows):
Custom PWL table (
pwl_params.jsonbuilt from realdata.jsonby non‑uniform quantiles):A quantized “default” lookup model (simulating input/output quantized at 1/128):
Full model output level (Conv+ReLU+Sigmoid branch, same circuit/logrows):
With the custom PWL lookup:
With the default lookup (quantized model):
So, for this toy model and data, the custom PWL improves end‑to‑end model accuracy by roughly 1–2 orders of magnitude, at essentially the same proving cost when
logrowsis chosen based onnum_rows.Testing
mock_custom_lookup_1l_sigmoidintests/integration_tests.rs, which runs the 1l_sigmoid example with a PWL file via--custom-lookup-path, then calibrate → compile → gen_witness → mock.custom_lookup_demo.ipynbintests/py_integration_tests.rsso the demo notebook is executed with the rest of the notebook suite.custom_lookup_pathset (gen_settings → compile → gen_witness → setup → prove → verify) succeeds; withcustom_lookup_path = None, behavior matches upstream (Sigmoid uses built‑in lookup).Documentation
docs/custom_lookup_table.mdwith the JSON schema, step‑by‑step guide, caveats (input must be within breakpoints, production/margin tip), and usage for Python/CLI.custom_lookup_demo.ipynbdocuments how to produce the table (generate_pwl_jsonwith uniform, curvature, quantile, or custom breakpoints) and the full EZKL pipeline.Review feedback addressed
examples/notebooks:custom_lookup_demo.ipynbwith full pipeline andgenerate_pwl_json.custom_lookup_demo.ipynbregistered intests/py_integration_tests.rs.mock_custom_lookup_1l_sigmoidintests/integration_tests.rs.log::warn!on first load.