Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding - #8

Open
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch
Open

Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding#8
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch

Conversation

@balbasty

@balbastybalbasty commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Two independent bugs in the *_kernel_add / *_kernel_sub wrappers, found
while porting these kernels to fastfields.
Both are verified against the current main (dc74377); a regression test that
fails before and passes after is included.

1. field_kernel_add/_ dispatch to impl.flow_kernel

field_kernel_add and field_kernel_add_ call impl.flow_kernel instead of
impl.field_kernel. This is a hard failure, not a silent one: the backend
signature is

flow_kernel(out, bound, voxel_size, absolute, membrane, bending, shears, div, op='')

— 8 required positional parameters — and the call site passes 7, so all four
public entry points raise immediately:

>>> field_kernel_add(2, inp, 0.3, 1.0, 0.2, voxel_size=[1.5, 1.0])
TypeError: flow_kernel() missing 1 required positional argument: 'div'

bindings/cuda's flow_kernel declares the identical 8-positional signature,
so the CUDA path fails the same way (unverified at runtime — no GPU here). The
stale See `flow_kernel` docstrings on the two _add variants (every
sibling says See `field_kernel` ) are corrected too — they look like the
origin of the slip.

Worth noting for the test: field_kernel and flow_kernel produce
bit-identical channel-0 kernels under the default isotropic voxel size,
and only diverge once voxel_size is anisotropic. So even a version of this
call that had the right arity would still have gone unnoticed under a
default-argument test.

2. field_kernel_sub/_ and flow_kernel_sub/_ silently add

All four forward to their _add sibling without the trailing _sub=True flag,
so op is computed as 'add' and they add the kernel instead of subtracting
it. On current main (flow variants are reachable today; the field ones are
blocked by bug 1):

>>> torch.equal(flow_kernel_add(inp, ...), flow_kernel_sub(inp, ...))
True # should be False
>>> torch.allclose(flow_kernel_sub(zeros, ...), -flow_kernel(...))
False # returns +kernel

Every other _sub wrapper in these two modules — field_matvec_sub,
field_matvec_sub_, field_diag_sub, field_diag_sub_, and the four flow
equivalents — does pass True. These four are the only omissions.

Changes

  • _regularisers_fields.py: impl.flow_kernelimpl.field_kernel in
    field_kernel_add and field_kernel_add_; fix their docstrings; forward
    True from field_kernel_sub / field_kernel_sub_.
  • _regularisers_flows.py: forward True from flow_kernel_sub /
    flow_kernel_sub_.
  • tests/test_reg_field.py, tests/test_reg_flow.py: new
    test_kernel_add_sub, asserting kernel_add(inp) == inp + kernel and
    kernel_sub(inp) == inp - kernel for the out-of-place and in-place variants.
    The voxel size is deliberately anisotropic ([1.5, 1., 2.5][:dim]) so
    that a field-vs-flow cross-wiring cannot hide behind the isotropic
    coincidence described above.

Verified locally (CPU backend, bindings/cpp):

# on main, the two new tests only
12 failed # field: TypeError above; flow: the `sub` assertion
# with this change, both reg modules
36 passed

One thing I did not touch

My first draft of the flow test also covered shears/div. I dropped it
because it runs into what looks like a separate, pre-existing problem:
flow_kernel with shears/div on a spatial shape larger than the minimal
one does not seem to produce a centred stencil. On current main:

>>> flow_kernel([5], shears=0.5, div=0.7, voxel_size=[1.5]) # shape (5,1,1)
[-1.7, -1.7, -1.7, -1.7, 3.4]

I would have expected a centred 3-tap stencil zero-padded to length 5, and the
voxel_size scaling also appears to be absent (2*shears + div == 1.7, with no
1/1.5²). The existing test_kernel only exercises shears/div at the
minimal [3]*dim shape, so this regime is untested. I have not diagnosed
this properly and it is unrelated to the two bugs fixed here — flagging it only
so it is not lost. (Instantiating the kernel_lame'+'/'-' templates also
crashed cppyy in my environment, which may or may not be related.)


Found while porting jitfields to https://github.com/fastfields. I have opened a
companion issue listing the other jitfields-inherited defects turned up by that
port. Left unmerged for your review, as requested.

Workstream: claude-jitfields-to-fastfields


Generated by Claude Code

…tracting
Two independent bugs in the `*_kernel_add` / `*_kernel_sub` wrappers, found
while porting these kernels to fastfields (https://github.com/fastfields).
1. `field_kernel_add` and `field_kernel_add_` called `impl.flow_kernel`
instead of `impl.field_kernel`. This is a hard failure, not a silent one:
the backend signature is
flow_kernel(out, bound, voxel_size, absolute, membrane, bending,
shears, div, op='')
-- 8 required positional parameters -- and the call site passes 7, so all
four public entry points raised
TypeError: flow_kernel() missing 1 required positional argument: 'div'
The stale "See `flow_kernel`" docstrings on the two `_add` variants (every
sibling says "See `field_kernel`") look like the origin of the slip, and
are corrected too.
Note that `field_kernel` and `flow_kernel` produce bit-identical channel-0
kernels under the default isotropic voxel size and only diverge once
`voxel_size` is anisotropic -- so even a call with the right arity would
have gone unnoticed under a default-argument test.
2. `field_kernel_sub`, `field_kernel_sub_`, `flow_kernel_sub` and
`flow_kernel_sub_` forwarded to their `_add` sibling without the trailing
`_sub=True` flag, so `op` was computed as 'add' and they added the kernel
instead of subtracting it. Every other `_sub` wrapper in these two modules
(field/flow x matvec/diag x in-place/out-of-place) does pass `True`; these
four were the only omissions.
Adds `test_kernel_add_sub` to tests/test_reg_field.py and tests/test_reg_flow.py,
asserting `kernel_add(inp) == inp + kernel` and `kernel_sub(inp) == inp - kernel`
for the out-of-place and in-place variants. The voxel size is deliberately
anisotropic ([1.5, 1., 2.5][:dim]) so that a field-vs-flow cross-wiring cannot
hide behind the isotropic coincidence described above.
Before: 12 failed (field: TypeError; flow: the `sub` assertion).
After: 36 passed across both reg test modules.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
Sign up for freeto 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.

2 participants

@balbasty@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all \u003cpre\u003e\u003ccode\u003e blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks"); } } catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); } })(); (function(){ try { var __m = "github.com"; var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding - #8

Open
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch
Open

Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding#8
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch

Conversation

@balbasty

@balbastybalbasty commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Two independent bugs in the *_kernel_add / *_kernel_sub wrappers, found
while porting these kernels to fastfields.
Both are verified against the current main (dc74377); a regression test that
fails before and passes after is included.

1. field_kernel_add/_ dispatch to impl.flow_kernel

field_kernel_add and field_kernel_add_ call impl.flow_kernel instead of
impl.field_kernel. This is a hard failure, not a silent one: the backend
signature is

flow_kernel(out, bound, voxel_size, absolute, membrane, bending, shears, div, op='')

— 8 required positional parameters — and the call site passes 7, so all four
public entry points raise immediately:

>>> field_kernel_add(2, inp, 0.3, 1.0, 0.2, voxel_size=[1.5, 1.0])
TypeError: flow_kernel() missing 1 required positional argument: 'div'

bindings/cuda's flow_kernel declares the identical 8-positional signature,
so the CUDA path fails the same way (unverified at runtime — no GPU here). The
stale See `flow_kernel` docstrings on the two _add variants (every
sibling says See `field_kernel` ) are corrected too — they look like the
origin of the slip.

Worth noting for the test: field_kernel and flow_kernel produce
bit-identical channel-0 kernels under the default isotropic voxel size,
and only diverge once voxel_size is anisotropic. So even a version of this
call that had the right arity would still have gone unnoticed under a
default-argument test.

2. field_kernel_sub/_ and flow_kernel_sub/_ silently add

All four forward to their _add sibling without the trailing _sub=True flag,
so op is computed as 'add' and they add the kernel instead of subtracting
it. On current main (flow variants are reachable today; the field ones are
blocked by bug 1):

>>> torch.equal(flow_kernel_add(inp, ...), flow_kernel_sub(inp, ...))
True # should be False
>>> torch.allclose(flow_kernel_sub(zeros, ...), -flow_kernel(...))
False # returns +kernel

Every other _sub wrapper in these two modules — field_matvec_sub,
field_matvec_sub_, field_diag_sub, field_diag_sub_, and the four flow
equivalents — does pass True. These four are the only omissions.

Changes

  • _regularisers_fields.py: impl.flow_kernelimpl.field_kernel in
    field_kernel_add and field_kernel_add_; fix their docstrings; forward
    True from field_kernel_sub / field_kernel_sub_.
  • _regularisers_flows.py: forward True from flow_kernel_sub /
    flow_kernel_sub_.
  • tests/test_reg_field.py, tests/test_reg_flow.py: new
    test_kernel_add_sub, asserting kernel_add(inp) == inp + kernel and
    kernel_sub(inp) == inp - kernel for the out-of-place and in-place variants.
    The voxel size is deliberately anisotropic ([1.5, 1., 2.5][:dim]) so
    that a field-vs-flow cross-wiring cannot hide behind the isotropic
    coincidence described above.

Verified locally (CPU backend, bindings/cpp):

# on main, the two new tests only
12 failed # field: TypeError above; flow: the `sub` assertion
# with this change, both reg modules
36 passed

One thing I did not touch

My first draft of the flow test also covered shears/div. I dropped it
because it runs into what looks like a separate, pre-existing problem:
flow_kernel with shears/div on a spatial shape larger than the minimal
one does not seem to produce a centred stencil. On current main:

>>> flow_kernel([5], shears=0.5, div=0.7, voxel_size=[1.5]) # shape (5,1,1)
[-1.7, -1.7, -1.7, -1.7, 3.4]

I would have expected a centred 3-tap stencil zero-padded to length 5, and the
voxel_size scaling also appears to be absent (2*shears + div == 1.7, with no
1/1.5²). The existing test_kernel only exercises shears/div at the
minimal [3]*dim shape, so this regime is untested. I have not diagnosed
this properly and it is unrelated to the two bugs fixed here — flagging it only
so it is not lost. (Instantiating the kernel_lame'+'/'-' templates also
crashed cppyy in my environment, which may or may not be related.)


Found while porting jitfields to https://github.com/fastfields. I have opened a
companion issue listing the other jitfields-inherited defects turned up by that
port. Left unmerged for your review, as requested.

Workstream: claude-jitfields-to-fastfields


Generated by Claude Code

…tracting
Two independent bugs in the `*_kernel_add` / `*_kernel_sub` wrappers, found
while porting these kernels to fastfields (https://github.com/fastfields).
1. `field_kernel_add` and `field_kernel_add_` called `impl.flow_kernel`
instead of `impl.field_kernel`. This is a hard failure, not a silent one:
the backend signature is
flow_kernel(out, bound, voxel_size, absolute, membrane, bending,
shears, div, op='')
-- 8 required positional parameters -- and the call site passes 7, so all
four public entry points raised
TypeError: flow_kernel() missing 1 required positional argument: 'div'
The stale "See `flow_kernel`" docstrings on the two `_add` variants (every
sibling says "See `field_kernel`") look like the origin of the slip, and
are corrected too.
Note that `field_kernel` and `flow_kernel` produce bit-identical channel-0
kernels under the default isotropic voxel size and only diverge once
`voxel_size` is anisotropic -- so even a call with the right arity would
have gone unnoticed under a default-argument test.
2. `field_kernel_sub`, `field_kernel_sub_`, `flow_kernel_sub` and
`flow_kernel_sub_` forwarded to their `_add` sibling without the trailing
`_sub=True` flag, so `op` was computed as 'add' and they added the kernel
instead of subtracting it. Every other `_sub` wrapper in these two modules
(field/flow x matvec/diag x in-place/out-of-place) does pass `True`; these
four were the only omissions.
Adds `test_kernel_add_sub` to tests/test_reg_field.py and tests/test_reg_flow.py,
asserting `kernel_add(inp) == inp + kernel` and `kernel_sub(inp) == inp - kernel`
for the out-of-place and in-place variants. The voxel size is deliberately
anisotropic ([1.5, 1., 2.5][:dim]) so that a field-vs-flow cross-wiring cannot
hide behind the isotropic coincidence described above.
Before: 12 failed (field: TypeError; flow: the `sub` assertion).
After: 36 passed across both reg test modules.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
Sign up for freeto 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.

2 participants

@balbasty@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding - #8

Open
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch
Open

Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding#8
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch

Conversation

@balbasty

@balbastybalbasty commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Two independent bugs in the *_kernel_add / *_kernel_sub wrappers, found
while porting these kernels to fastfields.
Both are verified against the current main (dc74377); a regression test that
fails before and passes after is included.

1. field_kernel_add/_ dispatch to impl.flow_kernel

field_kernel_add and field_kernel_add_ call impl.flow_kernel instead of
impl.field_kernel. This is a hard failure, not a silent one: the backend
signature is

flow_kernel(out, bound, voxel_size, absolute, membrane, bending, shears, div, op='')

— 8 required positional parameters — and the call site passes 7, so all four
public entry points raise immediately:

>>> field_kernel_add(2, inp, 0.3, 1.0, 0.2, voxel_size=[1.5, 1.0])
TypeError: flow_kernel() missing 1 required positional argument: 'div'

bindings/cuda's flow_kernel declares the identical 8-positional signature,
so the CUDA path fails the same way (unverified at runtime — no GPU here). The
stale See `flow_kernel` docstrings on the two _add variants (every
sibling says See `field_kernel` ) are corrected too — they look like the
origin of the slip.

Worth noting for the test: field_kernel and flow_kernel produce
bit-identical channel-0 kernels under the default isotropic voxel size,
and only diverge once voxel_size is anisotropic. So even a version of this
call that had the right arity would still have gone unnoticed under a
default-argument test.

2. field_kernel_sub/_ and flow_kernel_sub/_ silently add

All four forward to their _add sibling without the trailing _sub=True flag,
so op is computed as 'add' and they add the kernel instead of subtracting
it. On current main (flow variants are reachable today; the field ones are
blocked by bug 1):

>>> torch.equal(flow_kernel_add(inp, ...), flow_kernel_sub(inp, ...))
True # should be False
>>> torch.allclose(flow_kernel_sub(zeros, ...), -flow_kernel(...))
False # returns +kernel

Every other _sub wrapper in these two modules — field_matvec_sub,
field_matvec_sub_, field_diag_sub, field_diag_sub_, and the four flow
equivalents — does pass True. These four are the only omissions.

Changes

  • _regularisers_fields.py: impl.flow_kernelimpl.field_kernel in
    field_kernel_add and field_kernel_add_; fix their docstrings; forward
    True from field_kernel_sub / field_kernel_sub_.
  • _regularisers_flows.py: forward True from flow_kernel_sub /
    flow_kernel_sub_.
  • tests/test_reg_field.py, tests/test_reg_flow.py: new
    test_kernel_add_sub, asserting kernel_add(inp) == inp + kernel and
    kernel_sub(inp) == inp - kernel for the out-of-place and in-place variants.
    The voxel size is deliberately anisotropic ([1.5, 1., 2.5][:dim]) so
    that a field-vs-flow cross-wiring cannot hide behind the isotropic
    coincidence described above.

Verified locally (CPU backend, bindings/cpp):

# on main, the two new tests only
12 failed # field: TypeError above; flow: the `sub` assertion
# with this change, both reg modules
36 passed

One thing I did not touch

My first draft of the flow test also covered shears/div. I dropped it
because it runs into what looks like a separate, pre-existing problem:
flow_kernel with shears/div on a spatial shape larger than the minimal
one does not seem to produce a centred stencil. On current main:

>>> flow_kernel([5], shears=0.5, div=0.7, voxel_size=[1.5]) # shape (5,1,1)
[-1.7, -1.7, -1.7, -1.7, 3.4]

I would have expected a centred 3-tap stencil zero-padded to length 5, and the
voxel_size scaling also appears to be absent (2*shears + div == 1.7, with no
1/1.5²). The existing test_kernel only exercises shears/div at the
minimal [3]*dim shape, so this regime is untested. I have not diagnosed
this properly and it is unrelated to the two bugs fixed here — flagging it only
so it is not lost. (Instantiating the kernel_lame'+'/'-' templates also
crashed cppyy in my environment, which may or may not be related.)


Found while porting jitfields to https://github.com/fastfields. I have opened a
companion issue listing the other jitfields-inherited defects turned up by that
port. Left unmerged for your review, as requested.

Workstream: claude-jitfields-to-fastfields


Generated by Claude Code

…tracting
Two independent bugs in the `*_kernel_add` / `*_kernel_sub` wrappers, found
while porting these kernels to fastfields (https://github.com/fastfields).
1. `field_kernel_add` and `field_kernel_add_` called `impl.flow_kernel`
instead of `impl.field_kernel`. This is a hard failure, not a silent one:
the backend signature is
flow_kernel(out, bound, voxel_size, absolute, membrane, bending,
shears, div, op='')
-- 8 required positional parameters -- and the call site passes 7, so all
four public entry points raised
TypeError: flow_kernel() missing 1 required positional argument: 'div'
The stale "See `flow_kernel`" docstrings on the two `_add` variants (every
sibling says "See `field_kernel`") look like the origin of the slip, and
are corrected too.
Note that `field_kernel` and `flow_kernel` produce bit-identical channel-0
kernels under the default isotropic voxel size and only diverge once
`voxel_size` is anisotropic -- so even a call with the right arity would
have gone unnoticed under a default-argument test.
2. `field_kernel_sub`, `field_kernel_sub_`, `flow_kernel_sub` and
`flow_kernel_sub_` forwarded to their `_add` sibling without the trailing
`_sub=True` flag, so `op` was computed as 'add' and they added the kernel
instead of subtracting it. Every other `_sub` wrapper in these two modules
(field/flow x matvec/diag x in-place/out-of-place) does pass `True`; these
four were the only omissions.
Adds `test_kernel_add_sub` to tests/test_reg_field.py and tests/test_reg_flow.py,
asserting `kernel_add(inp) == inp + kernel` and `kernel_sub(inp) == inp - kernel`
for the out-of-place and in-place variants. The voxel size is deliberately
anisotropic ([1.5, 1., 2.5][:dim]) so that a field-vs-flow cross-wiring cannot
hide behind the isotropic coincidence described above.
Before: 12 failed (field: TypeError; flow: the `sub` assertion).
After: 36 passed across both reg test modules.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
Sign up for freeto 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.

2 participants

@balbasty@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length \u003e 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding - #8

Open
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch
Open

Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding#8
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch

Conversation

@balbasty

@balbastybalbasty commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Two independent bugs in the *_kernel_add / *_kernel_sub wrappers, found
while porting these kernels to fastfields.
Both are verified against the current main (dc74377); a regression test that
fails before and passes after is included.

1. field_kernel_add/_ dispatch to impl.flow_kernel

field_kernel_add and field_kernel_add_ call impl.flow_kernel instead of
impl.field_kernel. This is a hard failure, not a silent one: the backend
signature is

flow_kernel(out, bound, voxel_size, absolute, membrane, bending, shears, div, op='')

— 8 required positional parameters — and the call site passes 7, so all four
public entry points raise immediately:

>>> field_kernel_add(2, inp, 0.3, 1.0, 0.2, voxel_size=[1.5, 1.0])
TypeError: flow_kernel() missing 1 required positional argument: 'div'

bindings/cuda's flow_kernel declares the identical 8-positional signature,
so the CUDA path fails the same way (unverified at runtime — no GPU here). The
stale See `flow_kernel` docstrings on the two _add variants (every
sibling says See `field_kernel` ) are corrected too — they look like the
origin of the slip.

Worth noting for the test: field_kernel and flow_kernel produce
bit-identical channel-0 kernels under the default isotropic voxel size,
and only diverge once voxel_size is anisotropic. So even a version of this
call that had the right arity would still have gone unnoticed under a
default-argument test.

2. field_kernel_sub/_ and flow_kernel_sub/_ silently add

All four forward to their _add sibling without the trailing _sub=True flag,
so op is computed as 'add' and they add the kernel instead of subtracting
it. On current main (flow variants are reachable today; the field ones are
blocked by bug 1):

>>> torch.equal(flow_kernel_add(inp, ...), flow_kernel_sub(inp, ...))
True # should be False
>>> torch.allclose(flow_kernel_sub(zeros, ...), -flow_kernel(...))
False # returns +kernel

Every other _sub wrapper in these two modules — field_matvec_sub,
field_matvec_sub_, field_diag_sub, field_diag_sub_, and the four flow
equivalents — does pass True. These four are the only omissions.

Changes

  • _regularisers_fields.py: impl.flow_kernelimpl.field_kernel in
    field_kernel_add and field_kernel_add_; fix their docstrings; forward
    True from field_kernel_sub / field_kernel_sub_.
  • _regularisers_flows.py: forward True from flow_kernel_sub /
    flow_kernel_sub_.
  • tests/test_reg_field.py, tests/test_reg_flow.py: new
    test_kernel_add_sub, asserting kernel_add(inp) == inp + kernel and
    kernel_sub(inp) == inp - kernel for the out-of-place and in-place variants.
    The voxel size is deliberately anisotropic ([1.5, 1., 2.5][:dim]) so
    that a field-vs-flow cross-wiring cannot hide behind the isotropic
    coincidence described above.

Verified locally (CPU backend, bindings/cpp):

# on main, the two new tests only
12 failed # field: TypeError above; flow: the `sub` assertion
# with this change, both reg modules
36 passed

One thing I did not touch

My first draft of the flow test also covered shears/div. I dropped it
because it runs into what looks like a separate, pre-existing problem:
flow_kernel with shears/div on a spatial shape larger than the minimal
one does not seem to produce a centred stencil. On current main:

>>> flow_kernel([5], shears=0.5, div=0.7, voxel_size=[1.5]) # shape (5,1,1)
[-1.7, -1.7, -1.7, -1.7, 3.4]

I would have expected a centred 3-tap stencil zero-padded to length 5, and the
voxel_size scaling also appears to be absent (2*shears + div == 1.7, with no
1/1.5²). The existing test_kernel only exercises shears/div at the
minimal [3]*dim shape, so this regime is untested. I have not diagnosed
this properly and it is unrelated to the two bugs fixed here — flagging it only
so it is not lost. (Instantiating the kernel_lame'+'/'-' templates also
crashed cppyy in my environment, which may or may not be related.)


Found while porting jitfields to https://github.com/fastfields. I have opened a
companion issue listing the other jitfields-inherited defects turned up by that
port. Left unmerged for your review, as requested.

Workstream: claude-jitfields-to-fastfields


Generated by Claude Code

…tracting
Two independent bugs in the `*_kernel_add` / `*_kernel_sub` wrappers, found
while porting these kernels to fastfields (https://github.com/fastfields).
1. `field_kernel_add` and `field_kernel_add_` called `impl.flow_kernel`
instead of `impl.field_kernel`. This is a hard failure, not a silent one:
the backend signature is
flow_kernel(out, bound, voxel_size, absolute, membrane, bending,
shears, div, op='')
-- 8 required positional parameters -- and the call site passes 7, so all
four public entry points raised
TypeError: flow_kernel() missing 1 required positional argument: 'div'
The stale "See `flow_kernel`" docstrings on the two `_add` variants (every
sibling says "See `field_kernel`") look like the origin of the slip, and
are corrected too.
Note that `field_kernel` and `flow_kernel` produce bit-identical channel-0
kernels under the default isotropic voxel size and only diverge once
`voxel_size` is anisotropic -- so even a call with the right arity would
have gone unnoticed under a default-argument test.
2. `field_kernel_sub`, `field_kernel_sub_`, `flow_kernel_sub` and
`flow_kernel_sub_` forwarded to their `_add` sibling without the trailing
`_sub=True` flag, so `op` was computed as 'add' and they added the kernel
instead of subtracting it. Every other `_sub` wrapper in these two modules
(field/flow x matvec/diag x in-place/out-of-place) does pass `True`; these
four were the only omissions.
Adds `test_kernel_add_sub` to tests/test_reg_field.py and tests/test_reg_flow.py,
asserting `kernel_add(inp) == inp + kernel` and `kernel_sub(inp) == inp - kernel`
for the out-of-place and in-place variants. The voxel size is deliberately
anisotropic ([1.5, 1., 2.5][:dim]) so that a field-vs-flow cross-wiring cannot
hide behind the isotropic coincidence described above.
Before: 12 failed (field: TypeError; flow: the `sub` assertion).
After: 36 passed across both reg test modules.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
Sign up for freeto 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.

2 participants

@balbasty@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding - #8

Open
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch
Open

Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding#8
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch

Conversation

@balbasty

@balbastybalbasty commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Two independent bugs in the *_kernel_add / *_kernel_sub wrappers, found
while porting these kernels to fastfields.
Both are verified against the current main (dc74377); a regression test that
fails before and passes after is included.

1. field_kernel_add/_ dispatch to impl.flow_kernel

field_kernel_add and field_kernel_add_ call impl.flow_kernel instead of
impl.field_kernel. This is a hard failure, not a silent one: the backend
signature is

flow_kernel(out, bound, voxel_size, absolute, membrane, bending, shears, div, op='')

— 8 required positional parameters — and the call site passes 7, so all four
public entry points raise immediately:

>>> field_kernel_add(2, inp, 0.3, 1.0, 0.2, voxel_size=[1.5, 1.0])
TypeError: flow_kernel() missing 1 required positional argument: 'div'

bindings/cuda's flow_kernel declares the identical 8-positional signature,
so the CUDA path fails the same way (unverified at runtime — no GPU here). The
stale See `flow_kernel` docstrings on the two _add variants (every
sibling says See `field_kernel` ) are corrected too — they look like the
origin of the slip.

Worth noting for the test: field_kernel and flow_kernel produce
bit-identical channel-0 kernels under the default isotropic voxel size,
and only diverge once voxel_size is anisotropic. So even a version of this
call that had the right arity would still have gone unnoticed under a
default-argument test.

2. field_kernel_sub/_ and flow_kernel_sub/_ silently add

All four forward to their _add sibling without the trailing _sub=True flag,
so op is computed as 'add' and they add the kernel instead of subtracting
it. On current main (flow variants are reachable today; the field ones are
blocked by bug 1):

>>> torch.equal(flow_kernel_add(inp, ...), flow_kernel_sub(inp, ...))
True # should be False
>>> torch.allclose(flow_kernel_sub(zeros, ...), -flow_kernel(...))
False # returns +kernel

Every other _sub wrapper in these two modules — field_matvec_sub,
field_matvec_sub_, field_diag_sub, field_diag_sub_, and the four flow
equivalents — does pass True. These four are the only omissions.

Changes

  • _regularisers_fields.py: impl.flow_kernelimpl.field_kernel in
    field_kernel_add and field_kernel_add_; fix their docstrings; forward
    True from field_kernel_sub / field_kernel_sub_.
  • _regularisers_flows.py: forward True from flow_kernel_sub /
    flow_kernel_sub_.
  • tests/test_reg_field.py, tests/test_reg_flow.py: new
    test_kernel_add_sub, asserting kernel_add(inp) == inp + kernel and
    kernel_sub(inp) == inp - kernel for the out-of-place and in-place variants.
    The voxel size is deliberately anisotropic ([1.5, 1., 2.5][:dim]) so
    that a field-vs-flow cross-wiring cannot hide behind the isotropic
    coincidence described above.

Verified locally (CPU backend, bindings/cpp):

# on main, the two new tests only
12 failed # field: TypeError above; flow: the `sub` assertion
# with this change, both reg modules
36 passed

One thing I did not touch

My first draft of the flow test also covered shears/div. I dropped it
because it runs into what looks like a separate, pre-existing problem:
flow_kernel with shears/div on a spatial shape larger than the minimal
one does not seem to produce a centred stencil. On current main:

>>> flow_kernel([5], shears=0.5, div=0.7, voxel_size=[1.5]) # shape (5,1,1)
[-1.7, -1.7, -1.7, -1.7, 3.4]

I would have expected a centred 3-tap stencil zero-padded to length 5, and the
voxel_size scaling also appears to be absent (2*shears + div == 1.7, with no
1/1.5²). The existing test_kernel only exercises shears/div at the
minimal [3]*dim shape, so this regime is untested. I have not diagnosed
this properly and it is unrelated to the two bugs fixed here — flagging it only
so it is not lost. (Instantiating the kernel_lame'+'/'-' templates also
crashed cppyy in my environment, which may or may not be related.)


Found while porting jitfields to https://github.com/fastfields. I have opened a
companion issue listing the other jitfields-inherited defects turned up by that
port. Left unmerged for your review, as requested.

Workstream: claude-jitfields-to-fastfields


Generated by Claude Code

…tracting
Two independent bugs in the `*_kernel_add` / `*_kernel_sub` wrappers, found
while porting these kernels to fastfields (https://github.com/fastfields).
1. `field_kernel_add` and `field_kernel_add_` called `impl.flow_kernel`
instead of `impl.field_kernel`. This is a hard failure, not a silent one:
the backend signature is
flow_kernel(out, bound, voxel_size, absolute, membrane, bending,
shears, div, op='')
-- 8 required positional parameters -- and the call site passes 7, so all
four public entry points raised
TypeError: flow_kernel() missing 1 required positional argument: 'div'
The stale "See `flow_kernel`" docstrings on the two `_add` variants (every
sibling says "See `field_kernel`") look like the origin of the slip, and
are corrected too.
Note that `field_kernel` and `flow_kernel` produce bit-identical channel-0
kernels under the default isotropic voxel size and only diverge once
`voxel_size` is anisotropic -- so even a call with the right arity would
have gone unnoticed under a default-argument test.
2. `field_kernel_sub`, `field_kernel_sub_`, `flow_kernel_sub` and
`flow_kernel_sub_` forwarded to their `_add` sibling without the trailing
`_sub=True` flag, so `op` was computed as 'add' and they added the kernel
instead of subtracting it. Every other `_sub` wrapper in these two modules
(field/flow x matvec/diag x in-place/out-of-place) does pass `True`; these
four were the only omissions.
Adds `test_kernel_add_sub` to tests/test_reg_field.py and tests/test_reg_flow.py,
asserting `kernel_add(inp) == inp + kernel` and `kernel_sub(inp) == inp - kernel`
for the out-of-place and in-place variants. The voxel size is deliberately
anisotropic ([1.5, 1., 2.5][:dim]) so that a field-vs-flow cross-wiring cannot
hide behind the isotropic coincidence described above.
Before: 12 failed (field: TypeError; flow: the `sub` assertion).
After: 36 passed across both reg test modules.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
Sign up for freeto 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.

2 participants

@balbasty@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding - #8

Open
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch
Open

Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding#8
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch

Conversation

@balbasty

@balbastybalbasty commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Two independent bugs in the *_kernel_add / *_kernel_sub wrappers, found
while porting these kernels to fastfields.
Both are verified against the current main (dc74377); a regression test that
fails before and passes after is included.

1. field_kernel_add/_ dispatch to impl.flow_kernel

field_kernel_add and field_kernel_add_ call impl.flow_kernel instead of
impl.field_kernel. This is a hard failure, not a silent one: the backend
signature is

flow_kernel(out, bound, voxel_size, absolute, membrane, bending, shears, div, op='')

— 8 required positional parameters — and the call site passes 7, so all four
public entry points raise immediately:

>>> field_kernel_add(2, inp, 0.3, 1.0, 0.2, voxel_size=[1.5, 1.0])
TypeError: flow_kernel() missing 1 required positional argument: 'div'

bindings/cuda's flow_kernel declares the identical 8-positional signature,
so the CUDA path fails the same way (unverified at runtime — no GPU here). The
stale See `flow_kernel` docstrings on the two _add variants (every
sibling says See `field_kernel` ) are corrected too — they look like the
origin of the slip.

Worth noting for the test: field_kernel and flow_kernel produce
bit-identical channel-0 kernels under the default isotropic voxel size,
and only diverge once voxel_size is anisotropic. So even a version of this
call that had the right arity would still have gone unnoticed under a
default-argument test.

2. field_kernel_sub/_ and flow_kernel_sub/_ silently add

All four forward to their _add sibling without the trailing _sub=True flag,
so op is computed as 'add' and they add the kernel instead of subtracting
it. On current main (flow variants are reachable today; the field ones are
blocked by bug 1):

>>> torch.equal(flow_kernel_add(inp, ...), flow_kernel_sub(inp, ...))
True # should be False
>>> torch.allclose(flow_kernel_sub(zeros, ...), -flow_kernel(...))
False # returns +kernel

Every other _sub wrapper in these two modules — field_matvec_sub,
field_matvec_sub_, field_diag_sub, field_diag_sub_, and the four flow
equivalents — does pass True. These four are the only omissions.

Changes

  • _regularisers_fields.py: impl.flow_kernelimpl.field_kernel in
    field_kernel_add and field_kernel_add_; fix their docstrings; forward
    True from field_kernel_sub / field_kernel_sub_.
  • _regularisers_flows.py: forward True from flow_kernel_sub /
    flow_kernel_sub_.
  • tests/test_reg_field.py, tests/test_reg_flow.py: new
    test_kernel_add_sub, asserting kernel_add(inp) == inp + kernel and
    kernel_sub(inp) == inp - kernel for the out-of-place and in-place variants.
    The voxel size is deliberately anisotropic ([1.5, 1., 2.5][:dim]) so
    that a field-vs-flow cross-wiring cannot hide behind the isotropic
    coincidence described above.

Verified locally (CPU backend, bindings/cpp):

# on main, the two new tests only
12 failed # field: TypeError above; flow: the `sub` assertion
# with this change, both reg modules
36 passed

One thing I did not touch

My first draft of the flow test also covered shears/div. I dropped it
because it runs into what looks like a separate, pre-existing problem:
flow_kernel with shears/div on a spatial shape larger than the minimal
one does not seem to produce a centred stencil. On current main:

>>> flow_kernel([5], shears=0.5, div=0.7, voxel_size=[1.5]) # shape (5,1,1)
[-1.7, -1.7, -1.7, -1.7, 3.4]

I would have expected a centred 3-tap stencil zero-padded to length 5, and the
voxel_size scaling also appears to be absent (2*shears + div == 1.7, with no
1/1.5²). The existing test_kernel only exercises shears/div at the
minimal [3]*dim shape, so this regime is untested. I have not diagnosed
this properly and it is unrelated to the two bugs fixed here — flagging it only
so it is not lost. (Instantiating the kernel_lame'+'/'-' templates also
crashed cppyy in my environment, which may or may not be related.)


Found while porting jitfields to https://github.com/fastfields. I have opened a
companion issue listing the other jitfields-inherited defects turned up by that
port. Left unmerged for your review, as requested.

Workstream: claude-jitfields-to-fastfields


Generated by Claude Code

…tracting
Two independent bugs in the `*_kernel_add` / `*_kernel_sub` wrappers, found
while porting these kernels to fastfields (https://github.com/fastfields).
1. `field_kernel_add` and `field_kernel_add_` called `impl.flow_kernel`
instead of `impl.field_kernel`. This is a hard failure, not a silent one:
the backend signature is
flow_kernel(out, bound, voxel_size, absolute, membrane, bending,
shears, div, op='')
-- 8 required positional parameters -- and the call site passes 7, so all
four public entry points raised
TypeError: flow_kernel() missing 1 required positional argument: 'div'
The stale "See `flow_kernel`" docstrings on the two `_add` variants (every
sibling says "See `field_kernel`") look like the origin of the slip, and
are corrected too.
Note that `field_kernel` and `flow_kernel` produce bit-identical channel-0
kernels under the default isotropic voxel size and only diverge once
`voxel_size` is anisotropic -- so even a call with the right arity would
have gone unnoticed under a default-argument test.
2. `field_kernel_sub`, `field_kernel_sub_`, `flow_kernel_sub` and
`flow_kernel_sub_` forwarded to their `_add` sibling without the trailing
`_sub=True` flag, so `op` was computed as 'add' and they added the kernel
instead of subtracting it. Every other `_sub` wrapper in these two modules
(field/flow x matvec/diag x in-place/out-of-place) does pass `True`; these
four were the only omissions.
Adds `test_kernel_add_sub` to tests/test_reg_field.py and tests/test_reg_flow.py,
asserting `kernel_add(inp) == inp + kernel` and `kernel_sub(inp) == inp - kernel`
for the out-of-place and in-place variants. The voxel size is deliberately
anisotropic ([1.5, 1., 2.5][:dim]) so that a field-vs-flow cross-wiring cannot
hide behind the isotropic coincidence described above.
Before: 12 failed (field: TypeError; flow: the `sub` assertion).
After: 36 passed across both reg test modules.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
Sign up for freeto 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.

2 participants

@balbasty@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding - #8

Open
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch
Open

Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding#8
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch

Conversation

@balbasty

@balbastybalbasty commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Two independent bugs in the *_kernel_add / *_kernel_sub wrappers, found
while porting these kernels to fastfields.
Both are verified against the current main (dc74377); a regression test that
fails before and passes after is included.

1. field_kernel_add/_ dispatch to impl.flow_kernel

field_kernel_add and field_kernel_add_ call impl.flow_kernel instead of
impl.field_kernel. This is a hard failure, not a silent one: the backend
signature is

flow_kernel(out, bound, voxel_size, absolute, membrane, bending, shears, div, op='')

— 8 required positional parameters — and the call site passes 7, so all four
public entry points raise immediately:

>>> field_kernel_add(2, inp, 0.3, 1.0, 0.2, voxel_size=[1.5, 1.0])
TypeError: flow_kernel() missing 1 required positional argument: 'div'

bindings/cuda's flow_kernel declares the identical 8-positional signature,
so the CUDA path fails the same way (unverified at runtime — no GPU here). The
stale See `flow_kernel` docstrings on the two _add variants (every
sibling says See `field_kernel` ) are corrected too — they look like the
origin of the slip.

Worth noting for the test: field_kernel and flow_kernel produce
bit-identical channel-0 kernels under the default isotropic voxel size,
and only diverge once voxel_size is anisotropic. So even a version of this
call that had the right arity would still have gone unnoticed under a
default-argument test.

2. field_kernel_sub/_ and flow_kernel_sub/_ silently add

All four forward to their _add sibling without the trailing _sub=True flag,
so op is computed as 'add' and they add the kernel instead of subtracting
it. On current main (flow variants are reachable today; the field ones are
blocked by bug 1):

>>> torch.equal(flow_kernel_add(inp, ...), flow_kernel_sub(inp, ...))
True # should be False
>>> torch.allclose(flow_kernel_sub(zeros, ...), -flow_kernel(...))
False # returns +kernel

Every other _sub wrapper in these two modules — field_matvec_sub,
field_matvec_sub_, field_diag_sub, field_diag_sub_, and the four flow
equivalents — does pass True. These four are the only omissions.

Changes

  • _regularisers_fields.py: impl.flow_kernelimpl.field_kernel in
    field_kernel_add and field_kernel_add_; fix their docstrings; forward
    True from field_kernel_sub / field_kernel_sub_.
  • _regularisers_flows.py: forward True from flow_kernel_sub /
    flow_kernel_sub_.
  • tests/test_reg_field.py, tests/test_reg_flow.py: new
    test_kernel_add_sub, asserting kernel_add(inp) == inp + kernel and
    kernel_sub(inp) == inp - kernel for the out-of-place and in-place variants.
    The voxel size is deliberately anisotropic ([1.5, 1., 2.5][:dim]) so
    that a field-vs-flow cross-wiring cannot hide behind the isotropic
    coincidence described above.

Verified locally (CPU backend, bindings/cpp):

# on main, the two new tests only
12 failed # field: TypeError above; flow: the `sub` assertion
# with this change, both reg modules
36 passed

One thing I did not touch

My first draft of the flow test also covered shears/div. I dropped it
because it runs into what looks like a separate, pre-existing problem:
flow_kernel with shears/div on a spatial shape larger than the minimal
one does not seem to produce a centred stencil. On current main:

>>> flow_kernel([5], shears=0.5, div=0.7, voxel_size=[1.5]) # shape (5,1,1)
[-1.7, -1.7, -1.7, -1.7, 3.4]

I would have expected a centred 3-tap stencil zero-padded to length 5, and the
voxel_size scaling also appears to be absent (2*shears + div == 1.7, with no
1/1.5²). The existing test_kernel only exercises shears/div at the
minimal [3]*dim shape, so this regime is untested. I have not diagnosed
this properly and it is unrelated to the two bugs fixed here — flagging it only
so it is not lost. (Instantiating the kernel_lame'+'/'-' templates also
crashed cppyy in my environment, which may or may not be related.)


Found while porting jitfields to https://github.com/fastfields. I have opened a
companion issue listing the other jitfields-inherited defects turned up by that
port. Left unmerged for your review, as requested.

Workstream: claude-jitfields-to-fastfields


Generated by Claude Code

…tracting
Two independent bugs in the `*_kernel_add` / `*_kernel_sub` wrappers, found
while porting these kernels to fastfields (https://github.com/fastfields).
1. `field_kernel_add` and `field_kernel_add_` called `impl.flow_kernel`
instead of `impl.field_kernel`. This is a hard failure, not a silent one:
the backend signature is
flow_kernel(out, bound, voxel_size, absolute, membrane, bending,
shears, div, op='')
-- 8 required positional parameters -- and the call site passes 7, so all
four public entry points raised
TypeError: flow_kernel() missing 1 required positional argument: 'div'
The stale "See `flow_kernel`" docstrings on the two `_add` variants (every
sibling says "See `field_kernel`") look like the origin of the slip, and
are corrected too.
Note that `field_kernel` and `flow_kernel` produce bit-identical channel-0
kernels under the default isotropic voxel size and only diverge once
`voxel_size` is anisotropic -- so even a call with the right arity would
have gone unnoticed under a default-argument test.
2. `field_kernel_sub`, `field_kernel_sub_`, `flow_kernel_sub` and
`flow_kernel_sub_` forwarded to their `_add` sibling without the trailing
`_sub=True` flag, so `op` was computed as 'add' and they added the kernel
instead of subtracting it. Every other `_sub` wrapper in these two modules
(field/flow x matvec/diag x in-place/out-of-place) does pass `True`; these
four were the only omissions.
Adds `test_kernel_add_sub` to tests/test_reg_field.py and tests/test_reg_flow.py,
asserting `kernel_add(inp) == inp + kernel` and `kernel_sub(inp) == inp - kernel`
for the out-of-place and in-place variants. The voxel size is deliberately
anisotropic ([1.5, 1., 2.5][:dim]) so that a field-vs-flow cross-wiring cannot
hide behind the isotropic coincidence described above.
Before: 12 failed (field: TypeError; flow: the `sub` assertion).
After: 36 passed across both reg test modules.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
Sign up for freeto 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.

2 participants

@balbasty@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding - #8

Open
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch
Open

Fix field_kernel_* dispatching to flow_kernel, and kernel_sub silently adding#8
balbasty wants to merge 1 commit into
mainfrom
fix/field-kernel-add-dispatch

Conversation

@balbasty

@balbastybalbasty commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Two independent bugs in the *_kernel_add / *_kernel_sub wrappers, found
while porting these kernels to fastfields.
Both are verified against the current main (dc74377); a regression test that
fails before and passes after is included.

1. field_kernel_add/_ dispatch to impl.flow_kernel

field_kernel_add and field_kernel_add_ call impl.flow_kernel instead of
impl.field_kernel. This is a hard failure, not a silent one: the backend
signature is

flow_kernel(out, bound, voxel_size, absolute, membrane, bending, shears, div, op='')

— 8 required positional parameters — and the call site passes 7, so all four
public entry points raise immediately:

>>> field_kernel_add(2, inp, 0.3, 1.0, 0.2, voxel_size=[1.5, 1.0])
TypeError: flow_kernel() missing 1 required positional argument: 'div'

bindings/cuda's flow_kernel declares the identical 8-positional signature,
so the CUDA path fails the same way (unverified at runtime — no GPU here). The
stale See `flow_kernel` docstrings on the two _add variants (every
sibling says See `field_kernel` ) are corrected too — they look like the
origin of the slip.

Worth noting for the test: field_kernel and flow_kernel produce
bit-identical channel-0 kernels under the default isotropic voxel size,
and only diverge once voxel_size is anisotropic. So even a version of this
call that had the right arity would still have gone unnoticed under a
default-argument test.

2. field_kernel_sub/_ and flow_kernel_sub/_ silently add

All four forward to their _add sibling without the trailing _sub=True flag,
so op is computed as 'add' and they add the kernel instead of subtracting
it. On current main (flow variants are reachable today; the field ones are
blocked by bug 1):

>>> torch.equal(flow_kernel_add(inp, ...), flow_kernel_sub(inp, ...))
True # should be False
>>> torch.allclose(flow_kernel_sub(zeros, ...), -flow_kernel(...))
False # returns +kernel

Every other _sub wrapper in these two modules — field_matvec_sub,
field_matvec_sub_, field_diag_sub, field_diag_sub_, and the four flow
equivalents — does pass True. These four are the only omissions.

Changes

  • _regularisers_fields.py: impl.flow_kernelimpl.field_kernel in
    field_kernel_add and field_kernel_add_; fix their docstrings; forward
    True from field_kernel_sub / field_kernel_sub_.
  • _regularisers_flows.py: forward True from flow_kernel_sub /
    flow_kernel_sub_.
  • tests/test_reg_field.py, tests/test_reg_flow.py: new
    test_kernel_add_sub, asserting kernel_add(inp) == inp + kernel and
    kernel_sub(inp) == inp - kernel for the out-of-place and in-place variants.
    The voxel size is deliberately anisotropic ([1.5, 1., 2.5][:dim]) so
    that a field-vs-flow cross-wiring cannot hide behind the isotropic
    coincidence described above.

Verified locally (CPU backend, bindings/cpp):

# on main, the two new tests only
12 failed # field: TypeError above; flow: the `sub` assertion
# with this change, both reg modules
36 passed

One thing I did not touch

My first draft of the flow test also covered shears/div. I dropped it
because it runs into what looks like a separate, pre-existing problem:
flow_kernel with shears/div on a spatial shape larger than the minimal
one does not seem to produce a centred stencil. On current main:

>>> flow_kernel([5], shears=0.5, div=0.7, voxel_size=[1.5]) # shape (5,1,1)
[-1.7, -1.7, -1.7, -1.7, 3.4]

I would have expected a centred 3-tap stencil zero-padded to length 5, and the
voxel_size scaling also appears to be absent (2*shears + div == 1.7, with no
1/1.5²). The existing test_kernel only exercises shears/div at the
minimal [3]*dim shape, so this regime is untested. I have not diagnosed
this properly and it is unrelated to the two bugs fixed here — flagging it only
so it is not lost. (Instantiating the kernel_lame'+'/'-' templates also
crashed cppyy in my environment, which may or may not be related.)


Found while porting jitfields to https://github.com/fastfields. I have opened a
companion issue listing the other jitfields-inherited defects turned up by that
port. Left unmerged for your review, as requested.

Workstream: claude-jitfields-to-fastfields


Generated by Claude Code

…tracting
Two independent bugs in the `*_kernel_add` / `*_kernel_sub` wrappers, found
while porting these kernels to fastfields (https://github.com/fastfields).
1. `field_kernel_add` and `field_kernel_add_` called `impl.flow_kernel`
instead of `impl.field_kernel`. This is a hard failure, not a silent one:
the backend signature is
flow_kernel(out, bound, voxel_size, absolute, membrane, bending,
shears, div, op='')
-- 8 required positional parameters -- and the call site passes 7, so all
four public entry points raised
TypeError: flow_kernel() missing 1 required positional argument: 'div'
The stale "See `flow_kernel`" docstrings on the two `_add` variants (every
sibling says "See `field_kernel`") look like the origin of the slip, and
are corrected too.
Note that `field_kernel` and `flow_kernel` produce bit-identical channel-0
kernels under the default isotropic voxel size and only diverge once
`voxel_size` is anisotropic -- so even a call with the right arity would
have gone unnoticed under a default-argument test.
2. `field_kernel_sub`, `field_kernel_sub_`, `flow_kernel_sub` and
`flow_kernel_sub_` forwarded to their `_add` sibling without the trailing
`_sub=True` flag, so `op` was computed as 'add' and they added the kernel
instead of subtracting it. Every other `_sub` wrapper in these two modules
(field/flow x matvec/diag x in-place/out-of-place) does pass `True`; these
four were the only omissions.
Adds `test_kernel_add_sub` to tests/test_reg_field.py and tests/test_reg_flow.py,
asserting `kernel_add(inp) == inp + kernel` and `kernel_sub(inp) == inp - kernel`
for the out-of-place and in-place variants. The voxel size is deliberately
anisotropic ([1.5, 1., 2.5][:dim]) so that a field-vs-flow cross-wiring cannot
hide behind the isotropic coincidence described above.
Before: 12 failed (field: TypeError; flow: the `sub` assertion).
After: 36 passed across both reg test modules.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
Sign up for freeto 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.

2 participants

@balbasty@claude