Add custom noise model support to compile_canvas_to_stim

[masa10-f]

Summary

• Extract compile_canvas_to_pattern as a reusable function returning (Pattern, BaseGraphState, node_map)
• Add noise_models parameter to compile_canvas_to_stim for injecting custom graphqomb.NoiseModel instances
• Make legacy parameters (p_depol_after_clifford, p_before_meas_flip) optional

Test plan

• Added tests for custom noise model injection
• Added test to verify legacy and noise_models params cannot be mixed
• All existing tests pass

🤖 Generated with Claude Code

[claude]

Pull Request Review: Add custom noise model support to compile_canvas_to_stim

Summary

This PR introduces custom noise model support to the compile_canvas_to_stim function by:

1. Extracting compile_canvas_to_pattern as a reusable function
2. Adding a noise_models parameter for custom graphqomb.NoiseModel instances
3. Making legacy noise parameters optional for backward compatibility

Code Quality ✅

Strengths:

• Clean refactoring with clear separation of concerns
• Well-documented functions with comprehensive NumPy-style docstrings
• Proper use of type hints including TYPE_CHECKING guard for forward references
• Good backward compatibility by making legacy parameters optional with None defaults
• Tests follow existing project patterns

Structure:

• The extraction of compile_canvas_to_pattern provides good reusability for users who want the pattern without Stim compilation
• The function signatures are well-designed with keyword-only argument for noise_models (using *)

Potential Issues & Recommendations

🔴 Critical: Missing Validation Logic

The PR documents that legacy parameters "Cannot be combined with noise_models", and includes a test expecting a ValueError when both are used together. However, the actual validation code is missing from compile_canvas_to_stim.

Current code (lspattern/compiler.py:154-155):

pattern, _graph, node_map = compile_canvas_to_pattern(canvas)
# No validation here!

Required fix:
Add validation before calling compile_canvas_to_pattern:

# Validate that legacy and noise_models params are not mixed
if noise_models is not None and (p_depol_after_clifford is not None or p_before_meas_flip is not None):
    msg = "Legacy noise parameters cannot be used together with noise_models"
    raise ValueError(msg)

This validation might be implemented in graphqomb.stim_compile, but it is better practice to validate at the API boundary for clearer error messages.

Performance ✅

No performance concerns. The refactoring does not add overhead—just reorganizes existing logic.

Security ✅

No security concerns identified.

Test Coverage ⚠️

Existing tests:

• ✅ Custom noise models are accepted
• ✅ Custom noise appears in output circuit
• ✅ Mixed legacy + noise_models raises ValueError

Recommended additions:

• Test with multiple noise models in the sequence
• Test that None values for legacy params work correctly
• Regression test: ensure existing code paths still work with legacy params

API Design ✅

Excellent choices:

• Keyword-only argument for noise_models prevents accidental positional usage
• Optional parameters with None defaults maintain backward compatibility
• Clear separation between pattern compilation and Stim compilation
• Type hints use Sequence (immutable interface) rather than list

Verdict

Approve with required changes. The design is solid and the refactoring is clean, but the validation logic must be added before merge (unless it is already in graphqomb, which should be verified).

Required before merge:

1. Add validation to ensure legacy and noise_models params are mutually exclusive, OR verify that graphqomb.stim_compile handles this

Recommended improvements:

1. Add test coverage for multiple simultaneous noise models
2. Add example usage to docstring
3. Consider documenting when users should use compile_canvas_to_pattern vs compile_canvas_to_stim

Great work on maintaining backward compatibility while extending functionality! 🎉

[Copilot]

Pull request overview

This PR extends the Stim compilation pipeline to support injecting custom graphqomb.NoiseModel instances, while also extracting the core “canvas → pattern” compilation into a reusable helper.

Changes:

• Extract compile_canvas_to_pattern(canvas) -> (Pattern, BaseGraphState, node_map) from the existing Stim compilation flow.
• Extend compile_canvas_to_stim with a noise_models parameter for custom noise injection.
• Make legacy noise parameters optional and add tests covering custom noise injection and argument-mixing rejection.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
lspattern/compiler.py	Refactors compilation into a reusable pattern compiler and adds noise_models support to Stim compilation.
tests/test_custom_noise_models.py	Adds tests validating custom noise model injection and enforcing mutual exclusivity with legacy noise params.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.