-
Notifications
You must be signed in to change notification settings - Fork 634
Unify DASC public analysis validation #2395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -454,14 +454,16 @@ def test_analysis_arguments_fail_at_the_public_boundary(invalid_storage_dtype): | |
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("epsilon", [1.0, []]) | ||
| @pytest.mark.parametrize("epsilon", [1.0, True, [], 10**1000, torch.tensor([1e-3, 2e-3])]) | ||
| def test_analysis_rejects_invalid_epsilon_at_the_public_boundary(epsilon): | ||
| """Normalize invalid epsilon values to the public ValueError contract.""" | ||
| with pytest.raises(ValueError, match=r"epsilon must be finite and in \(0, 1\)"): | ||
| mtss.analyze_gdn_decay(TinyGatedDeltaNetForCausalLM(), epsilon=epsilon) # type: ignore[arg-type] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("static_gate_input", [torch.nan, []]) | ||
| @pytest.mark.parametrize( | ||
| "static_gate_input", [torch.nan, True, [], 10**1000, torch.tensor([-0.3, -0.2])] | ||
| ) | ||
| def test_analysis_rejects_invalid_static_gate_input_at_the_public_boundary(static_gate_input): | ||
| """Normalize invalid static gate values to the public ValueError contract.""" | ||
| with pytest.raises(ValueError, match="static_gate_input must be finite"): | ||
|
|
@@ -471,6 +473,52 @@ def test_analysis_rejects_invalid_static_gate_input_at_the_public_boundary(stati | |
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("argument", "value", "message"), | ||
| [ | ||
| ("epsilon", [], r"epsilon must be finite and in \(0, 1\)"), | ||
| ("epsilon", True, r"epsilon must be finite and in \(0, 1\)"), | ||
| ("epsilon", torch.nan, r"epsilon must be finite and in \(0, 1\)"), | ||
| ("epsilon", 10**1000, r"epsilon must be finite and in \(0, 1\)"), | ||
| ("epsilon", torch.tensor([1e-3, 2e-3]), r"epsilon must be finite and in \(0, 1\)"), | ||
| ("static_gate_input", [], "static_gate_input must be finite"), | ||
| ("static_gate_input", True, "static_gate_input must be finite"), | ||
| ("static_gate_input", torch.nan, "static_gate_input must be finite"), | ||
| ("static_gate_input", 10**1000, "static_gate_input must be finite"), | ||
| ("static_gate_input", torch.tensor([-0.3, -0.2]), "static_gate_input must be finite"), | ||
| ], | ||
|
Comment on lines
+478
to
+489
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [SUGGESTION] Both parametrizations use
Adding the non-finite rows keeps the two entry points' contracts symmetric in the test suite: @pytest.mark.parametrize(
("argument", "value", "message"),
[
("epsilon", [], r"epsilon must be finite and in \(0, 1\)"),
("epsilon", torch.nan, r"epsilon must be finite and in \(0, 1\)"),
("static_gate_input", [], "static_gate_input must be finite"),
("static_gate_input", torch.nan, "static_gate_input must be finite"),
],
)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in #2396. Direct horizon-computation tests now cover non-finite epsilon and static_gate_input, in addition to wrong types and oversized integers. |
||
| ) | ||
| def test_horizon_computation_rejects_invalid_public_arguments(argument, value, message): | ||
| """Use the same public argument contract for direct horizon computation.""" | ||
| kwargs = {argument: value} | ||
| with pytest.raises(ValueError, match=message): | ||
| mtss.compute_gdn_decay_horizons( | ||
| torch.tensor([0.0]), | ||
| torch.tensor([0.0]), | ||
| **kwargs, # type: ignore[arg-type] | ||
| ) | ||
|
|
||
|
|
||
| def test_horizon_computation_ignores_the_default_device(): | ||
| """Keep CPU horizon analysis independent of PyTorch's ambient allocation device.""" | ||
| a_log = torch.tensor([0.0]) | ||
| dt_bias = torch.tensor([0.0]) | ||
| with torch.device("meta"): | ||
| horizons = mtss.compute_gdn_decay_horizons(a_log, dt_bias) | ||
| assert horizons.device.type == "cpu" | ||
|
|
||
|
|
||
| def test_policy_lifecycle_ignores_the_default_device(): | ||
| """Keep calibration and checkpoint metadata validation on their declared CPU path.""" | ||
| model = TinyGatedDeltaNetForCausalLM() | ||
| with torch.device("meta"): | ||
| calibrated = mtss.calibrate(model, _config(wmax_candidates=[7]), [_candidate(7)]) | ||
| state = mto.modelopt_state(calibrated) | ||
| policy = mtss.export_policy(calibrated) | ||
| assert state["modelopt_state_dict"][0][0] == "dasc" | ||
| assert policy["layers"]["linear_attn"]["static_horizons"] | ||
|
|
||
|
|
||
| def test_bf16_storage_round_trip_loaded_in_fp32_preserves_policy(): | ||
| """Accept BF16-rounded values after a checkpoint loader materializes FP32 tensors.""" | ||
| model = mtss.calibrate( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Avoid repeated validation in the per-module analysis path.
analyze_gdn_decayvalidates the arguments at its public boundary._analyze_gdn_modulesthen callscompute_gdn_decay_horizonsonce per module, which repeats the same validation for every module.Keep the public-boundary checks. Route internal calls through a private computation helper that assumes validated arguments. Preserve validation for other public callers.
As per path instructions: “validate external arguments once at the public boundary and avoid redundant internal checks.”
🤖 Prompt for AI Agents
Source: Path instructions