Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CRITICAL] double-multiplication of initial weight in single-pattern-source fluence output #212

Closed
fangq opened this issue Feb 27, 2024 · 3 comments
Assignees

Comments

@fangq
Copy link
Owner

fangq commented Feb 27, 2024

@ShijieYan helped debugging the issue reported by Haohui Zhang in this mailing list thread

https://groups.google.com/g/mcx-users/c/KEH754XnJdY

he noticed that our single-pattern simulations have been generating incorrect outputs at least since 4 years ago (v2020 and onward, may be even earlier).

in 2020, a similar bug was fixed for diffuse transmittance c04bff5, but somehow we did not apply this fix to the volumetric fluence output.

if you use photon-sharing (multiple patterns) or if you just compute diffuse transmittance, your results were not impacted by this bug.

But if you are using the fluence output from a single pattern or pattern3d source, unfortunately mcx was giving you wrong results.

This is a critical bug.

@fangq
Copy link
Owner Author

fangq commented Feb 28, 2024

detailed information on this bug was described in the below mailing list announcement:
https://groups.google.com/g/mcx-users/c/l-glzQk1UgU

quoted below

What version was affected

we found that this bug appeared in MCX and MCXCL v2020, could be even earlier (although Shijie believes it was not there when we completed photon-sharing between 2019-2020)

What type of simulations were affected

all single-pattern simulations using 'pattern' or 'pattern3d' source type (excluding multi-pattern simulations using photon-sharing) with non-binary sources have been giving incorrect volumetric outputs (fluence/flux/energy)

photon-sharing based simulations, surface diffuse-reflectance outputs and detected photon outputs were not affected. other source types (fourier etc) were not affected.

simulations using binary (0-1) single pattern/pattern3d were not affected

What happened

our code mistakenly multiplied the photons launch weight twice when saving fluence/energy deposition. This effectively make the simulated pattern squared. In other words, if you simulate a pattern [0.1,0.2;0.5,1], mcx/mcxcl effectively simulated [0.01,0.04;0.25,1].

we discovered the same issue in Aug 2020, but was only fixed it in diffuse reflectance outputs (c04bff5); but somehow we missed the fluence output.

How to fix

If you happen to use non-binary single-pattern simulation in your work, please immediately update your mcx/mcxlab/mcxcl/mcxlabcl to the Github version at

https://mcx.space/nightly/github/
We sincerely apologize for the inaccurate results mcx/mcxcl produced. Please rerun your simulations and verify if your results were affected.

If you use pmcx, I have already updated pmcx to v0.2.12 after fixing this bug. Please upgrade your pmcx from pypi (https://pypi.org/project/pmcx/).

fangq added a commit to fangq/mcxcl that referenced this issue Feb 28, 2024
@Giovannatc
Copy link

Hello Fang! Does this bug affect mmc simulations?

@fangq
Copy link
Owner Author

fangq commented Feb 29, 2024

@Giovannatc, no, it does not.

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

No branches or pull requests

2 participants