feat: make OpenDP optional when mocking FLAME core#20
Open
reneraab wants to merge 1 commit into
Open
Conversation
OpenDP is only needed for local FLAME-core mocking during tests, and in practice only when an analyst runs local StarModel tests with DP turned on. This commit moves OpenDP into the optional mockdp dependency group and updates the mock behavior to emit a warning instead of failing when that dependency is missing.
📝 WalkthroughWalkthroughThis PR makes OpenDP an optional dependency and adds graceful degradation. The project now declares OpenDP in an optional Poetry group rather than a required dependency, and the mock SDK's local differential privacy feature logs a warning and proceeds without DP if OpenDP is unavailable. ChangesOpenDP Optional Dependency with Graceful Degradation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@flame/utils/mock_flame_core.py`:
- Around line 271-275: Validate local_dp before computing the Laplace scale: in
the code path that computes scale = local_dp['sensitivity'] /
local_dp['epsilon'] (used when creating laplace_mech and applying it to result),
first check that 'sensitivity' and 'epsilon' keys exist in local_dp and that
epsilon is a non-zero positive number; if validation fails, handle it
deterministically (e.g., raise a clear ValueError or log and abort/skip DP
noise) so KeyError/ZeroDivisionError cannot propagate out of submit_final_result
/ the Laplace noise application. Ensure the check occurs right before computing
scale and that the error handling path provides a clear message including which
key is invalid.
- Around line 12-13: Change the broad ImportError handler so only a missing
top-level opendp disables the feature: catch ModuleNotFoundError and check
e.name == "opendp" before setting USE_OPENDP = False, and re-raise any other
import exceptions; additionally harden the local DP scale calculation that uses
local_dp['sensitivity'] / local_dp['epsilon'] by validating local_dp contains
the 'sensitivity' and 'epsilon' keys and that epsilon is non-zero (raise or log
and skip the DP submission on invalid input) where the scale is computed,
referencing the USE_OPENDP symbol and the local_dp dict/scale computation to
locate the changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ba5d9ae4-9300-4f9e-b459-5d6bc14eafa5
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
flame/utils/mock_flame_core.pypyproject.toml
Comment on lines
+12
to
+13
| except ImportError: | ||
| USE_OPENDP = False |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="flame/utils/mock_flame_core.py"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE"
exit 1
fi
echo "=== Lines 1-80 ==="
sed -n '1,80p' "$FILE" | nl -ba -w2 -s':'
echo "=== Lines 240-320 ==="
sed -n '240,320p' "$FILE" | nl -ba -w2 -s':'
echo "=== Search for USE_OPENDP ==="
rg -n "USE_OPENDP" "$FILE" || true
echo "=== Search for make_laplace / local_dp usage ==="
rg -n "make_laplace|local_dp|sensitivity|epsilon|absolute_distance|ZeroDivisionError|KeyError" "$FILE" || trueRepository: PrivateAIM/python-sdk-patterns
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="flame/utils/mock_flame_core.py"
echo "=== Existence check ==="
ls -la "$FILE" || (echo "Missing: $FILE"; exit 1)
echo "=== Lines 1-120 ==="
sed -n '1,120p' "$FILE" | cat -n -ba
echo "=== Lines 240-340 ==="
sed -n '240,340p' "$FILE" | cat -n -ba
echo "=== Search for USE_OPENDP ==="
rg -n "USE_OPENDP" "$FILE" || true
echo "=== Search for DP/scale logic ==="
rg -n "make_laplace|local_dp|sensitivity|epsilon|absolute_distance" "$FILE" || trueRepository: PrivateAIM/python-sdk-patterns
Length of output: 283
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="flame/utils/mock_flame_core.py"
echo "=== OpenDP import block (lines 1-80) ==="
awk '{printf "%4d:%s\n", NR, $0}' "$FILE" | sed -n '1,80p'
echo "=== USE_OPENDP occurrences ==="
rg -n "USE_OPENDP" "$FILE" || true
echo "=== DP/scale logic region (lines 240-340) ==="
awk '{printf "%4d:%s\n", NR, $0}' "$FILE" | sed -n '240,340p'
echo "=== Search for local_dp and make_laplace usage ==="
rg -n "local_dp|make_laplace|sensitivity|epsilon" "$FILE" || trueRepository: PrivateAIM/python-sdk-patterns
Length of output: 9773
Narrow the OpenDP fallback to “opendp missing” and harden local DP scale calculation
- The OpenDP import uses
except ImportError: USE_OPENDP = False, which can silently disable DP and trigger the misleading “mockdp dependency group is not installed” warning even whenopendpis present but incompatible; catch only the case where the top-levelopendppackage is missing (e.g.,ModuleNotFoundErrorwithe.name == "opendp"), and re-raise all other import failures. - The local DP Laplace scale is computed as
local_dp['sensitivity'] / local_dp['epsilon']with no validation; missing keys orepsilon == 0can raiseKeyError/ZeroDivisionErrorduring result submission—validate inputs (and log/skip/raise) before computingscale.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@flame/utils/mock_flame_core.py` around lines 12 - 13, Change the broad
ImportError handler so only a missing top-level opendp disables the feature:
catch ModuleNotFoundError and check e.name == "opendp" before setting USE_OPENDP
= False, and re-raise any other import exceptions; additionally harden the local
DP scale calculation that uses local_dp['sensitivity'] / local_dp['epsilon'] by
validating local_dp contains the 'sensitivity' and 'epsilon' keys and that
epsilon is non-zero (raise or log and skip the DP submission on invalid input)
where the scale is computed, referencing the USE_OPENDP symbol and the local_dp
dict/scale computation to locate the changes.
Comment on lines
+271
to
+275
| scale = local_dp['sensitivity'] / local_dp['epsilon'] # Laplace scale parameter | ||
| laplace_mech = make_laplace(input_domain=atom_domain(T=float), | ||
| input_metric=absolute_distance(T=float), | ||
| scale=scale) | ||
| result = laplace_mech(float(result)) |
There was a problem hiding this comment.
Validate local_dp before computing Laplace scale.
Line 271 can raise KeyError or ZeroDivisionError (epsilon missing/zero), which aborts submit_final_result.
Proposed fix
if USE_OPENDP:
enable_features("contrib")
- scale = local_dp['sensitivity'] / local_dp['epsilon'] # Laplace scale parameter
+ sensitivity = local_dp.get("sensitivity")
+ epsilon = local_dp.get("epsilon")
+ if sensitivity is None or epsilon in (None, 0):
+ self.flame_log(
+ "Invalid local_dp config. Expected non-zero 'epsilon' and provided 'sensitivity'. DP step will be skipped.",
+ log_type=LogTypeLiteral.WARNING.value
+ )
+ self.final_results_storage = result
+ self.__pop_logs__()
+ return {"result": "submitted"}
+ scale = sensitivity / epsilon # Laplace scale parameter
laplace_mech = make_laplace(input_domain=atom_domain(T=float),
input_metric=absolute_distance(T=float),
scale=scale)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@flame/utils/mock_flame_core.py` around lines 271 - 275, Validate local_dp
before computing the Laplace scale: in the code path that computes scale =
local_dp['sensitivity'] / local_dp['epsilon'] (used when creating laplace_mech
and applying it to result), first check that 'sensitivity' and 'epsilon' keys
exist in local_dp and that epsilon is a non-zero positive number; if validation
fails, handle it deterministically (e.g., raise a clear ValueError or log and
abort/skip DP noise) so KeyError/ZeroDivisionError cannot propagate out of
submit_final_result / the Laplace noise application. Ensure the check occurs
right before computing scale and that the error handling path provides a clear
message including which key is invalid.
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
1 participant
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.
OpenDP is only needed for local FLAME-core mocking during tests, and in practice only when an analyst runs local StarModel tests with DP turned on. This commit moves OpenDP into the optional mockdp dependency group and updates the mock behavior to emit a warning instead of failing when that dependency is missing.
Summary by CodeRabbit
Release Notes
mockdpgroup to enable local differential privacy features.