Repository navigation
experiments: signals lab round 1, which extra phone signals sharpen clearances - #59
Conversation
Four experiments for which extra phone signals would sharpen geometry: edge geometry, plane consistency (occlusion without LiDAR), drift anatomy and a sensor error budget. Pass criteria are fixed here, before any run. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- edge-geometry: along-wall error at edges is set by view angle. Within 15 degrees of face-on it is 0.9 in at p90, and past 30 degrees 20 in. One wall normal fitted near the meter, as the app does, adds s*psi*tan(angle) with psi at a median 2.2 degrees. - drift-anatomy: 92% of ARKit's error runs along the direction of travel. A return-to-meter residual can't bound it, and scale wanders 1 to 2% within a walk. - plane-consistency: photo agreement through the wall plane can't replace a depth test on phones without LiDAR (1.1 ft false, 9.2 ft missed). - sensor-budget: closed-form budgets drop dual-camera disparity, focus distance, the barometer and a meter cover of unknown make. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (35)
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 |
…hort front door The pass criteria stay in each README word for word, as pre-registered in 397507f. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…out the server's paths From #59: the wall plane's source (single raycast yaw 2.2° median, 6.7° p90), span photos, scale references in view, the meter anchor's pose over time, and an optional UWB distance stream. The server's repository is private, so validator-checks.md no longer names its internal files. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cde05e30a
ℹ️ 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".
| "p90_3d_in": p90(np.linalg.norm(err, axis=1)) / INCH, | ||
| "along_share_of_total_p90": p90(np.abs(along)) / p90(horiz), | ||
| } | ||
| true_range = np.linalg.norm(ref, axis=1) |
There was a problem hiding this comment.
Compute UWB ranges from the fixed meter anchor
For every sliding window that does not start at the physical meter, ref is an end-minus-window-start displacement, so this range and the subsequent radial correction simulate moving the ranging phone to each window's start. This does not test A4's stated scenario of a second phone left at the meter and can materially change the reported 20/30 ft residuals; use positions relative to one fixed meter origin, or restrict the analysis to windows that start there.
AGENTS.md reference: AGENTS.md:L36-L36
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Still open at experiments/drift-anatomy/run.py:271. A4 ranges use window displacement, so anchor moves. Design must use endpoint positions relative to one fixed physical meter and rotate/apply estimator corrections in that same frame. Existing UWB residuals do not support fixed-meter claims.
| [tool.pytest.ini_options] | ||
| testpaths = ["tests"] |
There was a problem hiding this comment.
Wire the new experiment suites into make check
This commit adds four independent pytest configurations and test directories, but make check still runs only the server, web, and iOS targets, and no workflow invokes these experiment suites. Consequently regressions in the geometry and grading code can merge without any of the added tests running; add experiment test targets to the repository check/CI path.
AGENTS.md reference: AGENTS.md:L54-L54
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Still open at Makefile:26. Four suites absent from make check and CI. CI edits forbidden and root Makefile outside assigned paths; Sam must authorize check/CI wiring.
|
|
||
| **Run:** `uv run python run.py`: 2 min, 0.9 GB. Needs the evals checkout at 190ed33 and its ETH3D and MoGe-2 caches. | ||
|
|
||
| **What it changes:** ask for each edge from within about 20° of face-on, record the tap's view angle, and fit the wall plane across the walk. |
There was a problem hiding this comment.
Link the plan-changing results from the evidence overview
These experiments all state concrete product or plan changes, but none is linked from docs/00-overview.md. The repository's experiment guide requires a line under “Evidence so far” whenever the product or plan relies on a result, so the authoritative overview remains stale and future work cannot trace its new capture and plane decisions back to these measurements. Add overview entries for the results that the plan now relies on.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Still open at experiments/edge-geometry/README.md:28. Plan-changing results still absent from overview. docs/00-overview.md outside worker scope; lead/Sam must add evidence entries after correcting invalid drift and scoring claims.
…the checks' limits From #22's review and Codex review: the eleven items are proposals, not contract; #59's numbers name their proxy, dataset and simulation; the retired sharpness and consent fields are not claimed to be covered; window operability is evidence for the rules; the boundary frame goes in a new field. In the proposed checks: positions only (not rotation), shape not alignment for depth, non-negative indices and finite vertices for the mesh, finiteness of every parsed number. The pointer no longer lists the intake routes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- Hazards: a huge JSON integer parses as an int, and math.isfinite raises on it; check integers against their field's range and report failures as problems. - Proposal 10: each anchor-pose sample needs its timestamp and epoch. - Proposal 11: the #59 result assumes a still second phone at a known place; the proposal requires that, a time per range and the range's uncertainty. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Note Review skipped Codemetrics is installed on this repository, but it hasn't been added to your team yet. To enable AI code reviews for this repo, go to your dashboard and add house-scanning-master to your team's repositories. |
| true_range = np.linalg.norm(ref, axis=1) | ||
| a4[dataset][ft] = {"before_p90_in": p90(horiz) / INCH} | ||
| for sigma in RANGE_SIGMAS_M: | ||
| noise = rng.normal(0.0, sigma, size=(RANGE_DRAWS, len(ref))) | ||
| measured = np.maximum(true_range[None] + noise, 0.0) |
There was a problem hiding this comment.
Range anchor moves A4 is meant to test a second phone left at the meter. Here, each simulated range runs from a walk window’s start to its end, so the anchor moves with every window. The reported UWB errors and failure verdict therefore do not measure the proposed fixed-meter setup.
Prompt To Fix With AI
This is a comment left during a code review.
Path: experiments/drift-anatomy/run.py
Line: 271-275
Comment:
**Range anchor moves** A4 is meant to test a second phone left at the meter. Here, each simulated range runs from a walk window’s start to its end, so the anchor moves with every window. The reported UWB errors and failure verdict therefore do not measure the proposed fixed-meter setup.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
The dev deploy defaults to none when that variable is unset. Setting it to static or jwt turns auth back on without a code change. Co-authored-by: hss-cloud-agent <hss-cloud-agent@users.noreply.github.com>
Four experiments asking which extra things the phone could capture to make clearance measurements more accurate. Pass criteria were committed in 397507f before any run. Everything runs on public data with laser or surveyed truth, or is closed-form physics. No app or server code changes.
edge-geometrydrift-anatomyplane-consistencysensor-budgetWhat this suggests, pending the manager's call:
Each folder runs with
uv run python run.py(orbudget.py), anduv run pytest -qpasses in each. The three data experiments import the evals harness read-only fromt3/evalsat 190ed33 (#12).Limits:
METHODS.md.🤖 Generated with Claude Code