[FEATURE] Add noise options for tactile sensors: hysteresis, dead taxels, probe gain

[Milotrince]

Description

• Schmitt-trigger hysteresis on ContactProbe with separate contact_threshold / release_threshold
• Viscoelastic hysteresis on ContactDepthProbe, KinematicTaxel, ProximityTaxel, ElastomerTaxel
• Spatial crosstalk on KinematicTaxel, share FFT code with ElastomerTaxel
• Per-taxel probe_gain applied to measured contact depth, with optional resample-on-reset (probe_gain_resample_range).
• Dead taxels — dead_taxel_probability + dead_taxel_value_range, resampled per scene.reset()
• Allow 0 radius probes to be used like padding, so that we can use FFT acceleration for non perfect grid patterns
• Fix debug_draw on tactile sensors when history_length>0
• Grid layout regularity validation, irregular-but-near-regular grids proceed on the FFT/conv path with averaged spacing, with a warning.

Related Issue

Merge first: #2922

Motivation and Context

• More realistic noise parameters to match real tactile sensors

How Has This Been / Can This Be Tested?

Added tests to test_sensors.py

Screenshots (if appropriate):

Screen.Recording.2026-06-09.at.01.18.50.mov

Checklist:

• I read the CONTRIBUTING document.
• I followed the Submitting Code Changes section of CONTRIBUTING document.
• I tagged the title correctly (including BUG FIX/FEATURE/MISC/BREAKING)
• I updated the documentation accordingly or no change is needed.
• I tested my changes and added instructions on how to test it for reviewers.
• I have added tests to cover my changes.
• All new and existing tests passed.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6ff7d64a3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[duburcqa]

Audit: PR #2813 — Tactile sensor noise/speedup

Scale: +4360 / −1000 across 12 files. This is really ~5 new sensor variants plus shared BVH/FFT/hysteresis infrastructure. It is genuinely careful work — the kernels are well-structured, the dual-radius GT/measured split is consistent, and the tests assert physics (gain
scaling, hysteresis overshoot D*(1+strength), crosstalk leakage/conservation, filler-probe zeros, SDF-vs-raycast parity) rather than just "runs". That said, you asked me to be picky, and there are real issues.

Blocking / correctness

1. _SIM_BVHS global cache leaks and risks stale hits — raycaster.py:49

_SIM_BVHS: dict[int, list[_SolverBVH]] = {}
This per-process dict is keyed by id(sim) and is never cleared (I grepped the whole PR — only written at :82, read at :60, no teardown anywhere, no hook in sensor_manager.py). Two problems:
- Leak: every scene ever built in a process keeps its collision/visual BVH GPU buffers alive forever. The previous code stored solver_bvhs on RaycasterSharedMetadata (freed with the scene). This is a lifetime regression — painful for test suites / notebooks that build
many scenes.
- id() reuse: if a sim is collected, a new sim can reuse the address and ensure_solver_bvhs will return the old sim's BVHs (wrong geometry, silently). Whether this can happen depends on whether the cached entries transitively pin the sim alive; either way one of {leak,
stale-hit} is guaranteed.

Fix: tie the BVH list to scene/sim lifetime (store it on a sim-owned object and have the sim drop it on destroy), or use a WeakValueDictionary/weakref-keyed structure with a cleanup hook. A module-global keyed by id() is the wrong ownership model.

2. getattr on internal options — tactile_shared.py:500-501

strength = float(getattr(self._options, "hysteresis_strength", 0.0))
tau = float(getattr(self._options, "hysteresis_tau", 0.0))
CLAUDE.md explicitly prohibits getattr/hasattr (the only sanctioned exception is external-library polymorphic objects — your own memory feedback_getattr_external_polymorphic). self._options is an internal Genesis options object. The justifying comment is also factually
wrong: it claims ContactProbe "isn't a viscoelastic target" and doesn't declare the fields — but ContactProbe (tactile.py:174) does inherit ViscoelasticHysteresisOptionsMixin, as do all five sensors that mix in ViscoelasticHysteresisMixin. The 0.0 fallback is dead code.
Replace with direct attribute access self._options.hysteresis_strength. Note probe.py already does the right thing nearby with isinstance(opts, TactileProbeSensorOptionsMixin), so the codebase is internally inconsistent here.

3. .cpu().numpy() — point_cloud_tactile.py:1895-1897

geom_starts = geom_starts_tensor.cpu().numpy()
geom_ns = geom_ns_tensor.cpu().numpy()
idx_np = geom_idx_tensor.cpu().numpy()
CLAUDE.md / your memory feedback_numba_cache + the data-access rules: never .cpu().numpy(), use tensor_to_array (already imported in this file). Three occurrences inside the _build_mask local closure.

4. ContactProbe docstring is misplaced and silently lost — tactile.py:176-194

The class body opens with def _probe_local_normal_required(...) (line 176) before the triple-quoted block (line 180). Because the method def is the first statement, that string is a no-op expression, not the class docstring — ContactProbe.__doc__ is None and the
documented probe_radius / contact_threshold / release_threshold params don't reach users. Move the method below the docstring (as ContactDepthProbe at :218 correctly does).

Should-fix (CLAUDE.md violations)

5. list[tuple] with positional fields instead of NamedTuple

CLAUDE.md: "Plain dict for packing attributes is prohibited. Strongly typed named data structures should be used instead, either via dataclasses or just simple NamedTuples." The same applies to bare positional tuples used as records. Two offenders, both
documented-by-comment because they're fragile:
- grid_fft_meta: list[tuple] — 8-field (sensor_idx, g_ny, g_nx, probe_start, cache_start, lambda_d, spacing_u, spacing_v), unpacked positionally and via meta[0] (point_cloud_tactile.py:1632,1641,1653).
- crosstalk_meta: list[tuple] — 7-field (g_ny, g_nx, probe_start, cache_start, strength, r_v, r_u) (kinematic_tactile.py:1164,1222).

This file already uses NamedTuple for KinematicTaxelData/ProximityTaxelData, so the pattern is right there. Convert these.

6. Duplicated closest_point_on_triangle — surface_distance_probe.py:38-95

_func_closest_point_on_triangle is a byte-for-byte duplicate of raycast_qd.closest_point_on_triangle (which this very PR adds and which kinematic_tactile/point_cloud_tactile import). CLAUDE.md: "There are code duplication everywhere. This should be avoided." Import the
shared one.

7. .squeeze() (no-arg) — surface_distance_probe.py:484-485

link_pos = self._link.get_pos(env_idx).squeeze()
link_quat = self._link.get_quat(env_idx).squeeze()
CLAUDE.md: prefer explicit indexing/[..., 0, :] over blanket squashing. No-arg .squeeze() is also a latent bug (a length-1 probe or env dim would collapse wrongly). Index the intended axis explicitly.

8. .cpu().numpy()-adjacent: pervasive np.asarray on internal data

tactile_shared.py alone has 11 np.asarray. Many are on data you own (e.g. get_mesh_geom_chunks re-wraps geom.init_verts/init_faces; line 65 does np.asarray(faces, dtype=np.int32) where faces is already an int32 ndarray from line 58/61 — fully redundant). CLAUDE.md:
"Don't use np.asarray unless strictly necessary, ie for public API with no control on the data source. For internal dev or private code path, it should NEVER be the case." Tighten to .astype(..., copy=False) or drop entirely on internal paths.

9. Direct numpy dtypes + inconsistency

The BVH builders mix gs.np_float/gs.np_int (correct per CLAUDE.md) with raw np.float32/np.int32/np.int64/np.float64 (e.g. tactile_shared.py:57-65,98-150, point_cloud_tactile.py:96,101,224-225). CLAUDE.md prohibits direct numpy dtype use except at enforced external
interfaces. Pick the gs.* aliases consistently.

10. Local-scope function — point_cloud_tactile.py:1893 _build_mask

Defined inside build(). CLAUDE.md discourages local functions "unless very well motivated." It's a non-trivial closure used twice; either inline both or make it a module helper (and fix the .cpu().numpy() from #3 while doing so).

11. Repeated next(e for e in ... if e.raycast_mask is None) — 5 sites

kinematic_tactile.py:1016,1467, point_cloud_tactile.py:2039,2123, plus the depth-probe path. Same "find the rigid (non-visual) BVH entry" logic duplicated. Per your feedback_avoid_noise_helpers, 5 uses crosses the threshold for a small generic accessor.

Nits / questions

- dead-taxel doc vs. behavior: options doc (tactile.py:107, probe.py metadata) says dead value is "a fresh per-channel uniform sample", but dead_taxel_values is (B, total_n_probes) — one sample per probe, broadcast to all 3 force/torque channels (probe.py:281). Either
the doc is wrong or the sampling is. Clarify.
- BVH stack push without bound guard: the per-chunk static BVH kernels (point_cloud_tactile.py:451-455,1299-1304,1459-1464, surface_distance_probe.py:328-333) push both children with no stack_idx < STACK_SIZE-2 check, unlike the global-rigid-BVH kernels which do guard.
Median-split keeps these balanced (depth ≈ log2(N/8) < 32), so it's safe in practice — but the asymmetry is a footgun if leaf size or build strategy ever changes. A one-line guard or an asserting comment would be worth it.
- _probe_local_normal_required (options.py:250, tactile.py:176/222): an argument-free bool method that reads like a property — CLAUDE.md says attribute-like methods should be properties. Minor.
- Commit hygiene: the 7 commits ("rebase update", "cleanup", "optimizations", empty bodies) should be squashed to a single tagged title on merge (matches your feedback_commit_style).
- Docstring history: _kernel_elastomer_surface_state_bvh:1184 says "Merges what was previously two kernels…" — CLAUDE.md feedback_comments_no_history: describe current state, not the refactor history.

What I did not fully verify

I read the kernels for structural correctness but did not independently re-derive the HydroShear FFT dilation math, the Schmitt-trigger latch timing against the ring-rotation order, or the crosstalk separability/normalization — I relied on the (good) physics assertions
in test_sensors.py for those. If you want, I can run the test suite on the cluster and/or trace one of those numerically to confirm.

---
Bottom line: the design is sound and well-tested, but #1 (BVH cache lifetime) is a genuine resource/correctness bug that should block, and #2–#4 are clear CLAUDE.md violations with one (#2) sitting on an incorrect justifying comment. The rest are quality cleanups
consistent with your stated conventions.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6e970db529

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Reject mixed contact-depth backends

When two sensors of the same tactile class set different contact_depth_query values, this class-wide assignment silently makes every earlier sensor use the last sensor's backend; for example the parity test instantiates ContactDepthProbe(..."sdf") followed by ContactDepthProbe(..."raycast"), so both probes are updated through the raycast branch instead of comparing SDF against raycast. Since the option is exposed per sensor and documented as requiring class-wide agreement, this should either store the backend per sensor or raise on conflicting non-None modes rather than letting build order change readings.

Useful? React with 👍 / 👎.