Skip to content

Fix low-mua fluence deposition in MMC raytracers#129

Open
lpattelli wants to merge 1 commit into
fangq:masterfrom
lpattelli:fix/low-mua-fluence-mmc
Open

Fix low-mua fluence deposition in MMC raytracers#129
lpattelli wants to merge 1 commit into
fangq:masterfrom
lpattelli:fix/low-mua-fluence-mmc

Conversation

@lpattelli

Copy link
Copy Markdown

Dear Prof. Fang,

I'm opening this PR in an attempt to mirror the logic for the low-mua fix that was introduced in mcx with fangq/mcx#164, fixing fluence deposition in the low-absorption limit.
In mmc, this seems to have more ramifications than the quick mcx fix.

If you have any feedback, I'm happy to iterate.

@fangq

fangq commented Jun 29, 2026

Copy link
Copy Markdown
Owner

@lpattelli, thanks for the patch

I reviewed your changes, I have a few questions:

  1. your low-absorption logic seems to be different from what is currently implemented in mcx.
    https://github.com/fangq/mcx/blob/v2025.10/src/mcx_core.cu#L2160-L2161

it seems to be similar to @epini's initially patch, but this approach was found to cause regression and as fixed later. it also uses an unnamed threshold constant 1e-3f - it should preferrably be a macro, if introducing this extra threshold is entirely necessary.

  1. I am very conservative when touching existing lines of code because they have been extensively tested, and thus considered "protected assets" of the code. any unintended behavior differences could leave existing functions broken. Your changes to the CPU backend, in mmc_raytrace.c seems more than surgical, I am wondering if it is necessary for those changes, aside from the formulation difference compared to mcx?

@lpattelli lpattelli force-pushed the fix/low-mua-fluence-mmc branch from ce46d76 to c2e1123 Compare June 29, 2026 15:37
@lpattelli lpattelli force-pushed the fix/low-mua-fluence-mmc branch from c2e1123 to d9bc2fe Compare June 29, 2026 15:49
@lpattelli

Copy link
Copy Markdown
Author

Thanks for catching this. Indeed I forgot how the discussion evolved after our initial patch, and that it needed subsequent fixes.
I have checked the latest version of mcx and amended the PR to match its current behavior:

fluence = (mua < EPS) ? w0 * pathlength : (w0 - w1) / mua

So the extra mua*L threshold and the unnamed 1e-3f constant are removed.

I also tried to replicate mcx's use of __fdividef, while OpenCL/CPU falls back on plain division because __fdividef is CUDA-specific.

For the CPU backend: the reason the diff is slightly larger is that the existing code uses the same variable, ww, first as absorbed energy and then as fluence-like output after division by mua. For mua==0, absorbed energy remains zero, but fluence should still receive the pathlength-limit contribution. The CPU changes are meant to separate those two meanings while preserving the existing absorbed-energy path unaltered.

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.

2 participants