Skip to content

fix: weight oversample_thick/polar/omega per-contribution, not on the running sum#5

Open
gcmullen wants to merge 1 commit into
mainfrom
fix/subpixel-oversampling
Open

fix: weight oversample_thick/polar/omega per-contribution, not on the running sum#5
gcmullen wants to merge 1 commit into
mainfrom
fix/subpixel-oversampling

Conversation

@gcmullen

Copy link
Copy Markdown
Collaborator

Summary

In the per-pixel accumulation, oversample_thick/polar/omega multiply the running
accumulator I by capture_fraction/polar/omega_pixel on every innermost iteration
(:3026-3028). Since I already holds all prior contributions, each factor
re-multiplies earlier contributions on every later iteration — it compounds
across the subpixel/source/phi/mosaic/thick loops.

Effect

  • oversample_thick: sum ratio vs the per-contribution result grows 2.3x -> 42x as
    detector_thicksteps goes 2 -> 25 (layer 1's contribution ends up scaled by
    cf_1cf_2...*cf_N).
  • oversample_omega: corr collapses to 0.34 under divergence + oversample.

Fix

Fold each factor into a per-contribution term before accumulating:

double I_contribution = F_cell*F_cell*F_latt*F_latt;
if(oversample_thick) I_contribution *= capture_fraction;
if(oversample_polar) I_contribution *= polar;
if(oversample_omega) I_contribution *= omega_pixel;
I += I_contribution;

Verification

  • Default (non-oversample) path: byte-identical (all flags off => I += I_contribution
    is identical to the old I += ...).
  • oversample_thick: sum ratio flat ~1.0 across thicksteps 2->25 (was 2.3->42x).
  • oversample_polar/omega: parity restored (omega 0.34 -> 1.0).

Scope

nanoBragg.c, +6/-4, one hunk. No API/CLI change. Proposal for your review.

oversample_thick/polar/omega multiply the per-pixel running accumulator I by
capture_fraction/polar/omega_pixel on every innermost iteration. Since I already
holds all prior contributions, each factor re-multiplies earlier contributions
on every later iteration, compounding across the subpixel/source/phi/mosaic/thick
loops (oversample_thick diverges 2.3x->42x as detector_thicksteps grows 2->25;
oversample_omega collapses corr to 0.34).

Fold each factor into a per-contribution term before accumulating, matching the
intended single weighting (and what the CUDA port does). Default (non-oversample)
path is byte-identical.
Sign up for free to 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.

1 participant