Alignment accuracy

[Artur-man]

No description provided.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 07e6cad8e0

ℹ️ 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]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 38ff84ace4

ℹ️ 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]

Skip empty Matte maps in the manual UI

For raw-image manual registrations with method == "Non-Rigid", the C++ path calls alignImagesTPS() and never fills accuracyMatte, so the new Matte map output is a 0x0 matrix. This plot treats any matrix as drawable and then indexes nrow(cur_alignment_image):1, which becomes 0:1 and raises a subscript error when the Matte map tab is opened; guard on nonzero dimensions or return NULL for that method.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 09a3cc69f2

ℹ️ 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]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 194e272cf5

ℹ️ 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]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4d7d18b184

ℹ️ 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]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d68a1e8578

ℹ️ 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]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e0c37390ef

ℹ️ 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]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b70e84da37

ℹ️ 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]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 798b990e82

ℹ️ 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]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2abeb0abf0

ℹ️ 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]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e2357050ba

ℹ️ 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]

Return a missing map for collapsed percentile ranges

When either overlap image is constant or sufficiently background-dominated that its 0.5th and 99.5th percentiles are equal, this branch throws Invalid fixed intensity range (with the equivalent failure for the moving image). Such inputs can still produce a registered image and the scalar Matte metric already represents this case as NaN, but the optional map now aborts the entire automated or manual registration; return an all-NaN map or fall back to a usable range instead.

Useful? React with 👍 / 👎.

[Artur-man]

@claude review, give me a list of missing modifications before merge

[Artur-man]

@claude review again, this time scan the entire package and check if there are things I missed to update in the entire package regarding this particular PR.

[claude]

Claude finished @Artur-man's task in 8m 36s —— View job

---

Package-wide review — what's still missing before merge

• Full PR diff vs origin/dev (21 files, +1972/−1072)
• Build plumbing: src/Makevars*, RcppExports, NAMESPACE, DESCRIPTION
• Docs: man/, roxygen, NEWS.md, docs/
• Tests: tests/testthat/test-registration.R
• Callers of changed/removed functions across R + C++
• C++ metric/accuracy code review

First, what's already handled — most of the earlier Codex feedback landed: is_faulty is now wired from getKeypointMetrics (automated_registration.cpp:642), the mask is built after the h.empty() guard (:622), fine metrics + matte map are recomputed after TPS (:714-715), the before.png/after.png debug writes are gone, MatteMIMap is chunked with counts-based percentiles instead of materialising every overlap pixel, keypoint labels are correct, Inlier Perc. is named for its unit, reshape2 is no longer used (so no DESCRIPTION change needed), and Makevars.in/Makevars.win need nothing since neither lists OBJECTS. RcppExports.{R,cpp} are correctly regenerated for accuracy_rawvector and generateOverlapMask.

Below is what I'd still want fixed, ordered by risk.

---

Blockers

1. NaN in the matte map makes the R post-processing throw — R/registration.R:3252 and :3758

tmp[tmp < 0] <- 0

MatteMIMap deliberately leaves unmeasured chunks as NaN (src/matte_mi.cpp:336, skipped at :372/:377). NaN < 0 is NA, and a logical subscript containing NA in a replacement raises NAs are not allowed in subscripted assignments. Any registration with partial overlap or a background-dominated border produces such chunks, so this fires on ordinary inputs and kills an otherwise successful alignment. Use tmp[!is.na(tmp) & tmp < 0] <- 0. Fix this →

2. MatteMIMap throws instead of returning a missing map — src/matte_mi.cpp:314, :327, :330

if (global.validPairs == 0U) throw std::invalid_argument("The mask contains no valid pixels.");
if (!isValidRange(fixedRange))  throw std::invalid_argument("Invalid fixed intensity range.");
if (!isValidRange(movingRange)) throw std::invalid_argument("Invalid moving intensity range.");

Zero overlap (bad homography, landmarks outside the field) and constant-intensity overlap (blank/background-only channel) are all reachable on real data, and each aborts the whole registration via Rcpp. The scalar MatteMI handles the same conditions by returning quiet_NaN() (:414, :422). The optional QC map should do the same — return an all-NaN map of the right dimensions. Fix this →

3. sd grid is measured on a double transform — src/automated_registration.cpp:627-641

im1Proc is warped by h at :627-631, and only then passed to getKeypointMetrics → checkMappedGridDistribution(im1Proc, h) (src/metrics.cpp:349), which builds a grid over im1Proc's extent and applies h again. So the grid is generated on the reference extent and transformed a second time. This is not just a wrong QC number — metrics.cpp:351-353 uses it to force Degenerate = 1.0, which now gates the TPS branch at :665. Capture the source size next to the mask generation at :623 and pass the pre-warp extent. Fix this →

4. SimpleITK path builds a malformed fine stats vector — R/registration.R:3127 and :3656

reg$alignment_stats$fine[names(tfx$alignment_metrics)] <- tfx$alignment_metrics

On this path $fine is scalar NA (C++ returns NULL for fine because run_TPS is false). Named assignment into an unnamed scalar yields a length-4 vector whose first name is "", while $coarse is length 3 (manual) or 10 (automated). tab$Fine <- cur_align_stats[["fine"]] at :3000/:3458 then errors on row-count mismatch the moment the Alignment Stat tab renders. Initialise fine as the full all-NA metrics_set instead of scalar NA. Fix this →

---

High

5. Three of the new keypoint metrics ignore the y axis. Point2fToDoubleVector (src/auxiliary.cpp:136-143) keeps only .x, so cppSD(std::vector<Point2f>&) (:230) is an x-only spread. That affects sd query kpts (>1?), sd ref. kpts (>1?) and sd grid (in [w,h]?) — and the Degenerate flag derived from them (metrics.cpp:345, :351). Vertically-distributed landmarks with similar x will be reported degenerate. Relatedly, metrics.cpp:340 prints them as x= / y= when they're actually query and ref. Fix this →

6. TPS-only mapping still reaches generateOverlapMask — R/registration.R:3874

For manual Method == "Non-Rigid" with BSpline (SimpleITK), initial_mapping[[1]][[1]] is a 0×0 matrix (only TPS keypoints exist). numericMatrixToMat produces an empty cv::Mat, h.rows != 2 sends it to warpPerspective (src/metrics.cpp:137), and OpenCV aborts before Elastix ever runs. Needs a guard or a TPS branch for the mask. Fix this →

7. NORM_MINMAX distorts every histogram before it is normalised — src/metrics.cpp:192, :236, :292-295

cv::normalize(hist1, hist1, 0, 1, cv::NORM_MINMAX);
hist1 /= cv::sum(hist1)[0];

The min-max step forces the least-occupied occupied bin to exactly 0 before the probability normalisation, so Entropy, jointEntropy, Intersection and Bhattacharyya are all computed from a distorted distribution. Once hist /= sum(hist) is there, the normalize call is both redundant and harmful — drop it in all three places. Fix this →

8. getAlignmentAccuracy assumes query and reference share dimensions — R/registration.R:4064-4100

query_info is computed at :4071 and never used; accuracy_rawvector is handed ref_info$width/ref_info$height for the ref, the query and the mask (:4096-4097). If the aligned image ever differs in size, imageToMat misreads the raw buffer. Worth an explicit dimension check rather than relying on every caller.

---

Medium — package-wide gaps (the "did I miss anything" part)

9. The non-interactive path computes the new QC and throws it away. getNonInteractiveRegistration (R/registration.R:4014-4040) calls the same computeAutomatedPairwiseTransform / computeManualPairwiseTransform but keeps only results$mapping. So every registerSpatialData(..., interactive = FALSE) pays for two full-resolution MatteMIMap passes plus two MatteMI passes inside getAlignmentMetrics and discards all of it — and programmatic users have no way to reach the metrics the PR adds. Either surface alignment_stats / matte_map in the registerSpatialData return value, or add a flag to skip them when non-interactive. This is the biggest genuine omission I found.

10. No test touches any of the new code. tests/testthat/test-registration.R is unchanged by this PR. Nothing asserts on alignment_stats names or lengths, matte-map dimensions, NaN handling, accuracy_rawvector, or generateOverlapMask. That matters more than usual here because R/auxiliary.R's .ALIGNMENT_ACCURACY_METRICS / .ALIGNMENT_KEYPOINT_METRICS are matched against C++ std::map keys behind a hard stop("There are missing accuracy metrics!") (:3263, :3771) — a one-character rename in metrics.cpp breaks registration at runtime with nothing to catch it. And since the SimpleITK test is skip_if_not_installed("SimpleITK") + skip_on_os("windows"), items 4, 6 and 12 are effectively untested. (The one CI run on this PR was cancelled, so there's no signal there either.)

11. Two exported functions changed signature with no deprecation and no NEWS.md entry. Both are in NAMESPACE:

• warpSimpleITKImage(query_image, mapping) dropped ref_image (R/registration.R:2773, man/warpSimpleITKImage.Rd:7). warpSimpleITKImage(ref_image = r, query_image = q, mapping = m) now errors, and positional calls silently bind the old query_image to mapping.
• getRcppManualRegistration inserted invert_query/invert_ref before method/nonrigid (:3158, man/getRcppManualRegistration.Rd:10-14), so getRcppManualRegistration(q, r, qp, rp, "Affine") now binds "Affine" to invert_query and silently runs the Homography path.

Either append the new arguments / keep a deprecated ref_image = NULL, or document both breaks in NEWS.md — right now NEWS.md only mentions the new accuracy metrics.

12. convertToSitkImage creates a vector image and then casts it to a scalar type — R/registration.R:2490

img_data <- magick::image_data(img, channels = "gray")   # 1 channel
SimpleITK::as.image(img_data, isVector = TRUE)

:3894 and :3896 then do SimpleITK::Cast(fixed, "sitkUInt8"), which asks for a scalar pixel type. SimpleITK's CastImageFilter does not support vector→scalar. Please verify with SimpleITK installed; if it does fail, this should be isVector = FALSE.

13. NULL assignment shifts the per-image QC lists — R/registration.R:2916, :2919

For the manual coordinate path manual_registeration_matrix returns a 2-element list, so matte_map/alignment_stats are NULL, and matte_map_list[[i]] <- NULL is a no-op. length(matte_map_list) then passes the guard at :2970 while matte_map_list[[i]] at :2971 (and alignment_stats_list[[i]] at :2995) is out of bounds — exactly the mixed image / image-free assay scenario NEWS.md advertises. Pre-fill with NA or assign via matte_map_list[i] <- list(results$matte_map).

14. Matte map orientation vs the returned image. accuracyMatte is built in im2Proc coordinates (automated_registration.cpp:658, :715) while im1Reg goes through reversepreprocessImage(..., flipflop_ref, rotate_ref) at :671/:721. With a rotated or flipped reference the map's hot/cold regions no longer line up with aligned_image, and a 90°/270° rotation swaps its dimensions too. Note dest_image (:785) is in processed orientation, so the two returned images disagree with each other — worth deciding explicitly which one the Matte tab overlays.

---

Low / cleanup

• docs/voltronobjects.html is unrelated churn and includes a new warning in the rendered output: A channel with name 'combined' already exists in this vrImage object. Overwriting ..., plus re-wrapped console widths. Nothing to do with alignment accuracy — I'd revert it here and handle that warning in its own PR.
• Commented-out debris: src/metrics.h:37-39 (dead generateOverlapMask Rcpp declaration), src/auxiliary.h:24 (dead matToMask overload), src/metrics.cpp:307-311, and R/registration.R:2985, 3251, 3757, 3877, 3885-3899, 3898.
• src/auxiliary.h has no trailing newline (\ No newline at end of file in the diff).
• Bitwise | where || is meant: src/metrics.cpp:111 and :351.
• src/accuracy.cpp returns an unnamed 3-element list consumed positionally at R/registration.R:3959-3971. Naming it (metrics, matte_map, overlay) makes the contract explicit.
• cppSD divides by n - 1 unguarded (src/auxiliary.cpp:227, :246) — 0 or 1 points gives NaN/Inf, which then flows into Degenerate.
• getAlignmentAccuracy (R/registration.R:4064) is the only function in that file with no roxygen block at all, not even @noRd.
• matToMask (src/auxiliary.cpp:68-81) sets dim = (rows, cols) while memcpy-ing row-major data, so the R matrix is the transpose; R/registration.R:3897 compensates with rev(dim(mask)). It works, but a comment on both sides would stop someone "fixing" one half.

---

Suggested merge gate: items 1–4 (each errors on ordinary input), then 5–7 (the reported numbers are wrong), then 9–11. I did not implement any of these — this was a review pass, as requested. Happy to take any subset if you want them applied.

I could not compile or run R CMD check / the test suite in this environment (no R toolchain available to the sandbox), so everything above is from reading the code. Item 12 in particular I'd like confirmed against a real SimpleITK install.
• branch dev_align_stats