Skip to content

Fix #215: [Model] EnsembleComputation#721

Merged
isPANN merged 4 commits intomainfrom
issue-215
Mar 21, 2026
Merged

Fix #215: [Model] EnsembleComputation#721
isPANN merged 4 commits intomainfrom
issue-215

Conversation

@GiggleLiu
Copy link
Copy Markdown
Contributor

Summary

  • Add an implementation plan for EnsembleComputation

Fixes #215

@GiggleLiu
Copy link
Copy Markdown
Contributor Author

Implementation Summary

Changes

  • Added the EnsembleComputation satisfaction model with validated construction, registry/variant metadata, prelude exports, and canonical example wiring.
  • Added pred create EnsembleComputation support, CLI help text, and CLI coverage for creation, validation, and --example.
  • Added model-focused tests plus a paper problem definition with background, worked example, and citation.

Deviations from Plan

  • Kept the paper example on the issue's 4-element instance, but used a smaller canonical --example instance so the example-db optimality check can solve it without an ILP reduction.

Open Questions

  • None.

@GiggleLiu
Copy link
Copy Markdown
Contributor Author

Agentic Review Report

Note: This PR currently has merge conflicts with main (mergeStateStatus=DIRTY, mergeable=CONFLICTING). These must be resolved before merging.

Structural Check

Structural Review: model EnsembleComputation

Structural Completeness

# Check Status
1 Model file exists PASS
2 inventory::submit! present PASS
3 #[derive(...Serialize, Deserialize)] on struct PASS
4 Problem trait impl PASS
5 SatisfactionProblem impl PASS
6 #[cfg(test)] + #[path = ...] test link PASS
7 Test file exists PASS
8 Test file has >= 3 test functions PASS
9 Registered in misc/mod.rs PASS
10 Re-exported in models/mod.rs PASS
11 declare_variants! entry exists PASS
12 CLI problem-name resolution PASS — registry-backed resolve_alias() requires no per-model hardcoded entry
13 CLI create support PASS
14 Canonical model example registered PASS
15 Paper display-name entry PASS
16 Paper problem-def block PASS
17 Blacklisted auto-generated files absent PASS

Build Status

  • make test: PASS
  • make clippy: PASS
  • make paper: PASS

Semantic Review

  • evaluate() correctness: OK — enforces config length, rejects future references via decode_operand, checks disjointness before union, and preserves the issue's j <= J semantics by accepting as soon as a valid prefix has produced every required subset.
  • dims() correctness: OK — vec![universe_size + budget; 2 * budget] matches the issue's flat operand encoding, with evaluate() filtering out future references.
  • Size getter consistency: OK — universe_size(), num_subsets(), and budget() line up with the schema and complexity metadata.
  • Weight handling: OK — N/A for this unweighted satisfaction problem.

Issue Compliance

# Check Status
1 Problem name matches issue OK
2 Mathematical definition matches OK
3 Problem type (sat) matches OK
4 Type parameters match OK — no generic parameters
5 Configuration space matches OK
6 Feasibility check matches OK
7 Objective function matches OK
8 Complexity matches OK

Summary

  • 17/17 structural checks passed
  • 8/8 issue compliance checks passed
  • Deterministic context note: the pre-generated implementation packet flagged files outside the current model whitelist (problemreductions-cli/..., src/lib.rs, src/models/mod.rs, src/models/misc/mod.rs). Those are real whitelist deviations and should be sanity-checked in final review, although they match current extension points for adding a user-facing model.

Quality Check

Quality Review

Design Principles

  • DRY: OK — operand decoding, disjointness checking, and union construction are factored into small helpers instead of duplicated inline logic.
  • KISS: OK — the model uses the simplest workable flat encoding from the issue and avoids extra abstraction.
  • HC/LC: OK — problem logic, CLI construction, tests, and paper wiring stay in the expected modules.

HCI (if CLI/MCP changed)

  • Error messages: OK — missing flags and invalid budgets are actionable, and the failing solve path at least suggests --solver brute-force.
  • Discoverability: ISSUE — pred create --example EnsembleComputation succeeds, but the next natural step pred solve <instance> fails under defaults because the model has no ILP/reduction path yet.
  • Consistency: OK — create/help wiring follows existing CLI patterns.
  • Least surprise: ISSUE — exposing a canonical example through pred create --example suggests a direct solve flow, but the default solver path dead-ends for this model.
  • Feedback: OK — the failing solve path gives a usable fallback hint.

Test Quality

  • Naive test detection: ISSUE
    • problemreductions-cli/tests/cli_tests.rs:7415 only checks pred create --example EnsembleComputation; there is no CLI coverage for solving that example, so the broken default pred solve path was not exercised.

Issues

Critical (Must Fix)

  • None.

Important (Should Fix)

  • The canonical EnsembleComputation example is not solvable via the default CLI path. Reproduction from the reviewed branch: pred create --example EnsembleComputation > /tmp/ensemble.json succeeds, but pred solve /tmp/ensemble.json fails with No reduction path from EnsembleComputation to ILP or ILP solver found no solution. Try --solver brute-force. Rerunning with --solver brute-force succeeds. This is a real user-path mismatch between --example and the default solve flow.
  • Add CLI regression coverage for the solve path in addition to the existing create/example coverage at problemreductions-cli/tests/cli_tests.rs:7415.

Minor (Nice to Have)

  • None.

Summary

  • The implementation itself looks coherent and the build/tests are clean, but the user-facing CLI path is incomplete: the new canonical example requires an explicit --solver brute-force flag that users only discover after a failed default solve.

Agentic Feature Tests

Feature: EnsembleComputation

Use case: A downstream CLI user discovers the new model from the catalog, inspects it, creates the canonical example, and tries to solve it.

  • Discoverability: Partial — pred list shows EnsembleComputation, pred show EnsembleComputation renders the schema/complexity correctly, and pred create --example EnsembleComputation emits a valid example instance.
  • Setup: PASS — the existing CLI binary built and ran successfully in the review environment.
  • Functionality: Partial — the default solve path failed; the brute-force fallback worked.
  • Expected vs Actual: Partial — expected the canonical example to be directly solvable from the default CLI flow, but in practice the user must add --solver brute-force after the default solve fails.
  • Blocked steps: None.
  • Friction points: The PR adds a canonical example and create coverage, but not a successful default solve path or test coverage for the required brute-force invocation.
  • Doc suggestions: Either make the expected solver choice explicit in user-facing docs/help for this model, or improve the default solve behavior for standalone problems with no reduction path.

Confirmed issues

  • Important — Reproduced in the reviewed branch:
    1. target/debug/pred create --example EnsembleComputation > /tmp/ensemble.json
    2. target/debug/pred solve /tmp/ensemble.json → fails with the no-ILP/reduction-path error and a --solver brute-force hint
    3. target/debug/pred solve /tmp/ensemble.json --solver brute-force → succeeds and returns a satisfying assignment

Generated by review-pipeline

… and ConsistencyOfDatabaseFrequencyTables/TimetableDesign

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@isPANN isPANN mentioned this pull request Mar 21, 2026
3 tasks
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 21, 2026

Codecov Report

❌ Patch coverage is 97.57282% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.62%. Comparing base (77577dd) to head (027323b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/models/misc/ensemble_computation.rs 96.45% 5 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main     #721    +/-   ##
========================================
  Coverage   97.61%   97.62%            
========================================
  Files         401      403     +2     
  Lines       49906    50112   +206     
========================================
+ Hits        48717    48923   +206     
  Misses       1189     1189            

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@isPANN isPANN merged commit a4e05bd into main Mar 21, 2026
5 checks passed
@GiggleLiu GiggleLiu deleted the issue-215 branch April 12, 2026 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Model] EnsembleComputation

2 participants