EngramConv.init has hardcoded dilation=3, not passed from EngramModule #15

New issue

Open

opened 2026-05-09 19:22:08 +02:00 by sleepy · 0 comments

sleepy commented

2026-05-09 19:22:08 +02:00

Owner

Problem

EngramConv.__init__ (engram.py:117) has dilation=3 as a hardcoded default:

def __init__(self, d: int, kernel_size: int = 4, dilation: int = 3):

But EngramModule.__init__ (engram.py:140) never passes dilation:

self.conv = EngramConv(d, kernel_size=conv_kernel, dilation=max(n_orders))

Wait — it does pass dilation=max(n_orders). But EngramConv.__init__'s signature has dilation=3 as default, and the call site passes it explicitly. So this is actually fine.

However, the comment in EngramModule says dilation=max(n_orders) but n_orders=(2,3), so dilation=3. This matches the default. The issue is that if someone changes n_orders to (2,), dilation becomes 2, but the default in EngramConv is still 3. The mismatch between default and actual usage is confusing.

Impact

Low — the call site passes dilation explicitly, so it works correctly
The hardcoded default of 3 is misleading if someone uses EngramConv directly without passing dilation

Action needed

Either:

Remove the dilation parameter from EngramConv.__init__ and always compute it in EngramModule, OR
Make the default match the most common usage (dilation=3 is fine if documented)

Files

tergent/engram.py:117, 140

## Problem `EngramConv.__init__` (engram.py:117) has `dilation=3` as a hardcoded default: ```python def __init__(self, d: int, kernel_size: int = 4, dilation: int = 3): ``` But `EngramModule.__init__` (engram.py:140) never passes `dilation`: ```python self.conv = EngramConv(d, kernel_size=conv_kernel, dilation=max(n_orders)) ``` Wait — it does pass `dilation=max(n_orders)`. But `EngramConv.__init__`'s signature has `dilation=3` as default, and the call site passes it explicitly. So this is actually fine. **However**, the comment in `EngramModule` says `dilation=max(n_orders)` but `n_orders=(2,3)`, so dilation=3. This matches the default. The issue is that if someone changes `n_orders` to `(2,)`, dilation becomes 2, but the default in `EngramConv` is still 3. The mismatch between default and actual usage is confusing. ## Impact - Low — the call site passes dilation explicitly, so it works correctly - The hardcoded default of 3 is misleading if someone uses `EngramConv` directly without passing dilation ## Action needed Either: - Remove the `dilation` parameter from `EngramConv.__init__` and always compute it in `EngramModule`, OR - Make the default match the most common usage (`dilation=3` is fine if documented) ## Files - `tergent/engram.py:117, 140`

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

sleepy/ternary#15

No description provided.

Rows
Columns

EngramConv.__init__ has hardcoded dilation=3, not passed from EngramModule #15

Problem

Impact

Action needed

Files

EngramConv.init has hardcoded dilation=3, not passed from EngramModule #15