fix: canonicalise translate/recolor parameters #14
Conversation
[S:ALG v1] translate/recolor int normalisation pass
WalkthroughIntroduces canonicalization and normalization of operation parameters (translate/recolor), adds alias compatibility (fill_value, color_map→mapping), updates public signatures and signature metadata, adjusts heuristic/search emitters, normalizes Episode programs on load, refactors caching keys, and adds tests for recolor/translate fixes plus a minor test tweak. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant DSL as dsl.apply_op
participant Canon as _canonical_params
participant Norm as _norm_params
participant Cache as OpCache
participant Op as Operation
Caller->>DSL: apply_op(op, params, a)
DSL->>Canon: canonicalize(params)
Canon-->>DSL: canon_params
DSL->>Norm: normalize(canon_params)
Norm-->>DSL: key
DSL->>Cache: get(key)
alt hit
Cache-->>DSL: result
DSL-->>Caller: result
else miss
DSL->>Op: execute(op, canon_params, a)
Op-->>DSL: result
DSL->>Cache: put(key, result)
DSL-->>Caller: result
end
sequenceDiagram
autonumber
actor Loader as Episode.from_dict
participant Data as raw data
participant Prog as programs[]
participant Norm as Normalizer
Loader->>Data: read programs
loop each program
Data->>Prog: (op, params)
Prog->>Norm: normalize(op, params)
alt op == "recolor"
Note over Norm: Use params.mapping or params.color_map<br/>Cast keys/values to int<br/>Produce {mapping: {...}}
else op == "translate"
Note over Norm: Coerce dy, dx, fill/fill_value to int<br/>If fill missing and fill_value present -> fill
else other
Note over Norm: pass-through
end
Norm-->>Prog: (op, norm_params)
end
Prog-->>Loader: normalized programs
Loader-->>Loader: construct Episode(programs=...)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
tylerbessire
deleted the
codex/outline-arc-training-and-evaluation-process
branch
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
arc_solver/dsl.py (1)
181-183: Align translate wrapper parameter order to (dy, dx).arc_solver/dsl.py: translate(a, dx, dy, ...) declares dx before dy but calls op_translate(a, dy, dx); op_translate and other APIs use (dy, dx) (op_translate: arc_solver/dsl.py:63; wrapper: arc_solver/dsl.py:181–183; grid.translate: arc_solver/grid.py:97; dsl_complete.translate: arc_solver/dsl_complete.py:42). Change the wrapper to def translate(a, dy: int = 0, dx: int = 0, fill_value: Optional[int] = None) and pass the args to op_translate in that same order.
🧹 Nitpick comments (5)
arc_solver/dsl.py (2)
135-144: Consider using frozenset for dict normalization.The current implementation converts dicts to sorted tuples for hashability, which works correctly. However, for mappings where order doesn't matter semantically, using
frozensetmight be more appropriate and potentially more efficient.Consider this alternative approach for better semantic clarity:
def _norm_params(params: Dict[str, Any]) -> Tuple[Tuple[str, Any], ...]: """Normalise parameters to a hashable tuple.""" items: List[Tuple[str, Any]] = [] for k, v in sorted(params.items()): if isinstance(v, dict): - items.append((k, tuple(sorted(v.items())))) + items.append((k, frozenset(v.items()))) else: items.append((k, v)) return tuple(items)
186-188: Consider deprecation warning for legacy parameter name.The
recolorwrapper still usescolor_mapas its parameter name while the internal implementation expectsmapping. While the canonicalization handles this transparently, consider adding a deprecation warning to guide users toward the new parameter name.def recolor(a: Array, color_map: Dict[int, int]) -> Array: """Recolour grid according to a mapping.""" + # Note: color_map parameter name is kept for backward compatibility + # but 'mapping' is the canonical parameter name internally return op_recolor(a, color_map)AGENTS.md (1)
482-496: Add language to fenced block to satisfy markdownlint (MD040).Specify a language for the code fence.
-``` +```text [X] Step 4.3 UPDATE - Recolor parameter mismatch fixed preventing training failures Date: 2025-09-12 Test Result: pytest tests/test_recolor_fix.py passed Notes: Standardised 'mapping' parameter across heuristics; episodic loader normalises keys [X] Step 4.3 UPDATE2 - Translate parameter mismatch fixed preventing training warnings Date: 2025-09-13 Test Result: pytest tests/test_translate_fix.py passed; python tools/train_guidance_on_arc.py --epochs 1 Notes: Canonicalised 'fill' parameter for translate; legacy 'fill_value' still accepted [X] Step 4.3 UPDATE3 - Translate/recolor params normalised to integers preventing training failures Date: 2025-09-13 Test Result: pytest tests/test_translate_fix.py tests/test_recolor_fix.py -q Notes: Episode loader and DSL cast dy/dx/fill and mapping entries to int</blockquote></details> <details> <summary>arc_solver/enhanced_search_complete.py (2)</summary><blockquote> `354-359`: **Use canonical 'fill' for translate parameters (avoid alias).** Aligns with PR objective and normalized cache keys; keeps params consistent across emitters. ```diff - param_combinations.append({'dx': dx, 'dy': dy, 'fill_value': 0}) + param_combinations.append({'dx': dx, 'dy': dy, 'fill': 0})
367-372: Cast recolor mapping keys/values to int when generating candidates.Ensures JSON-compatibility and matches Episode/DSL int-normalization. Also aligns with retrieved learning to standardize 'mapping'.
- param_combinations.append({"mapping": {old_color: new_color}}) + param_combinations.append({"mapping": {int(old_color): int(new_color)}})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
AGENTS.md(1 hunks)arc_solver/dsl.py(3 hunks)arc_solver/dsl_complete.py(3 hunks)arc_solver/enhanced_search_complete.py(1 hunks)arc_solver/heuristics_complete.py(4 hunks)arc_solver/neural/episodic.py(1 hunks)tests/test_beam_search.py(1 hunks)tests/test_recolor_fix.py(1 hunks)tests/test_translate_fix.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
arc_solver/neural/episodic.py
📄 CodeRabbit inference engine (AGENTS.md)
arc_solver/neural/episodic.py: Enhance episodic retrieval with AnalogicalReasoner class providing structural analogy search, solution mapping, and pattern abstraction
Upgrade episodic memory with hierarchical indexing and consolidation mechanisms
Files:
arc_solver/neural/episodic.py
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: tylerbessire/PUMA#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T05:16:41.579Z
Learning: Standardize recolor parameter key to mapping across heuristics; ensure episodic loader normalizes keys
📚 Learning: 2025-09-13T05:16:41.579Z
Learnt from: CR
PR: tylerbessire/PUMA#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T05:16:41.579Z
Learning: Applies to arc_solver/enhanced_search.py : Generate full parameter grid for recolor: use dictionaries with key mapping: {i: j} for all i != j
Applied to files:
arc_solver/enhanced_search_complete.pytests/test_recolor_fix.py
📚 Learning: 2025-09-13T05:16:41.579Z
Learnt from: CR
PR: tylerbessire/PUMA#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T05:16:41.579Z
Learning: Applies to arc_solver/search.py : Generate full parameter grid for recolor: use dictionaries with key mapping: {i: j} for all i != j
Applied to files:
arc_solver/enhanced_search_complete.py
📚 Learning: 2025-09-13T05:16:41.579Z
Learnt from: CR
PR: tylerbessire/PUMA#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T05:16:41.579Z
Learning: Standardize recolor parameter key to mapping across heuristics; ensure episodic loader normalizes keys
Applied to files:
tests/test_recolor_fix.py
📚 Learning: 2025-09-13T05:16:41.579Z
Learnt from: CR
PR: tylerbessire/PUMA#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T05:16:41.579Z
Learning: Canonicalize translate fill parameter to fill while accepting legacy fill_value
Applied to files:
arc_solver/dsl.py
🧬 Code graph analysis (5)
tests/test_beam_search.py (1)
arc_solver/grid.py (1)
to_array(37-39)
tests/test_translate_fix.py (3)
arc_solver/dsl.py (1)
apply_program(220-230)arc_solver/heuristics_complete.py (1)
detect_basic_transformations(74-109)arc_solver/neural/episodic.py (3)
Episode(34-117)to_dict(67-82)from_dict(85-117)
tests/test_recolor_fix.py (4)
arc_solver/grid.py (1)
to_array(37-39)arc_solver/dsl.py (1)
apply_program(220-230)arc_solver/heuristics_complete.py (1)
detect_color_patterns(198-224)arc_solver/neural/episodic.py (3)
Episode(34-117)to_dict(67-82)from_dict(85-117)
arc_solver/heuristics_complete.py (3)
arc_solver/dsl.py (1)
apply_program(220-230)arc_solver/dsl_complete.py (1)
apply_program(676-691)arc_solver/grid.py (1)
color_map(107-115)
arc_solver/dsl_complete.py (2)
arc_solver/dsl.py (2)
translate(181-183)recolor(186-188)arc_solver/grid.py (1)
color_map(107-115)
🪛 markdownlint-cli2 (0.17.2)
AGENTS.md
482-482: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (16)
arc_solver/dsl.py (3)
63-79: LGTM! Clean implementation of backward compatibility.The function signature elegantly maintains backward compatibility by introducing
fill_valueas a keyword-only alias while keepingfillas the primary parameter. The logic correctly prioritizesfilloverfill_valueand provides sensible default behavior.
119-133: Well-structured parameter canonicalization logic.The
_canonical_paramsfunction effectively normalizes legacy aliases and ensures consistent typing:
- Properly handles the
color_map→mappingalias for recolor operations- Correctly migrates
fill_value→fillfor translate operations- Ensures integer typing for all relevant parameters
This aligns with the learning about standardizing parameter keys across the codebase.
146-157: Robust caching implementation with proper canonicalization.The updated
apply_opfunction correctly:
- Canonicalizes parameters before caching
- Uses normalized parameters for cache key generation
- Maintains semantic correctness while improving cache hit rates
This ensures that operations with equivalent parameters (regardless of aliasing) will properly hit the cache.
arc_solver/neural/episodic.py (1)
91-104: Parameter normalization correctly implemented for episode deserialization.The changes add proper normalization for recolor and translate operations during episode reconstruction:
- Recolor normalization: Handles both legacy
color_mapand newmappingparameter names, ensuring integer keys/values- Translate normalization: Converts all parameters to integers and maps legacy
fill_valuetofillparameter- Consistent with PR objectives: Aligns with the goal to canonicalize translate/recolor parameters
This ensures Episodes can handle both legacy and canonical parameter formats while maintaining type consistency.
arc_solver/heuristics_complete.py (4)
107-107: Translate parameter name correctly updated to canonical form.The change from
fill_valuetofillaligns with the PR's objective to canonicalize translate parameters. This matches the parameter name changes in the DSL layer.
205-207: Recolor parameter standardization implemented correctly.The parameter name change from
color_maptomappingstandardizes the recolor operation across the codebase. The logic remains functionally equivalent while using the canonical parameter name.
327-329: Multi-step recolor operation correctly updated.The recolor operation in the multi-step detection path now consistently uses the
mappingparameter name, maintaining consistency with the single-step recolor changes.
500-501: Color mapping inference now ensures integer keys and values.The explicit casting to
int()ensures that color mappings always use integer types, which is important for consistent serialization and parameter handling across the system.tests/test_translate_fix.py (2)
14-24: Legacy alias support and canonical parameter detection correctly tested.The test validates that:
- The legacy
fill_valuealias continues to work for backward compatibility- The heuristics emit canonical parameter names (
fillinstead offill_value)This ensures the transition maintains backward compatibility while standardizing on canonical forms.
27-42: Episode serialization round-trip correctly tests parameter normalization.The property-based test validates that:
- Episodes can handle string-valued parameters during construction
- Serialization/deserialization normalizes parameters to integers
- The
fill_valuetofillalias mapping works correctly- Round-trip consistency is maintained
This comprehensive test ensures the parameter canonicalization works correctly in realistic scenarios.
tests/test_recolor_fix.py (2)
17-23: Recolor parameter standardization correctly tested.The test validates that:
detect_color_patternsreturns programs using the newmappingparameter nameapply_programcorrectly processes recolor operations with themappingparameterThis ensures consistency across the heuristics and execution layers.
26-40: Episode recolor serialization round-trip correctly tested.The property-based test validates that recolor mappings maintain integer type consistency through serialization/deserialization cycles. The test ensures that:
- Episodes preserve integer types in recolor mappings
- Round-trip serialization maintains functionality
- The mapping parameter is correctly handled throughout the process
arc_solver/dsl_complete.py (3)
42-68: Translate function signature successfully updated for parameter canonicalization.The changes implement several important improvements:
- Parameter order change:
dy, dxorder now matches the mathematical convention- Canonical parameter name: Uses
fillinstead offill_valueas the primary parameter- Backward compatibility: Maintains
fill_valueas a keyword-only parameter for legacy support- Proper precedence handling:
fill_valueis only used whenfillretains its default valueThis design ensures smooth migration while establishing the canonical interface.
106-116: Recolor function signature successfully updated for parameter standardization.The changes implement dual parameter support:
- Legacy support: Maintains
color_mapparameter for existing code- New canonical form: Introduces
mappingparameter as the preferred interface- Proper precedence:
mappingtakes precedence when both are provided- Type flexibility: Both parameters accept
Dict[int, int] | NoneThis ensures a smooth transition to the standardized parameter name while maintaining backward compatibility.
701-705: Operation signatures correctly updated to reflect canonical parameter names.The signature metadata now reflects the canonical parameter names:
translateuses['dy', 'dx', 'fill']recoloruses['mapping']This ensures consistency between the public API and the operation registry metadata.
tests/test_beam_search.py (1)
30-34: LGTM: clearer “no-solution” guard.Changing b to 1x2 makes the unsatisfiable shape change explicit for depth=1.
Summary
dy,dx,fill) and recolor mappings to integersTesting
pip install scipy -qpytest tests/test_translate_fix.py tests/test_recolor_fix.py tests/test_beam_search.py -qpytest tests -qpython tools/train_guidance_on_arc.py --epochs 1https://chatgpt.com/codex/tasks/task_e_68c489916a30832286cc8ff5ea78d65c
Summary by CodeRabbit
Bug Fixes
Documentation
Tests