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

compact implementation of ring source, close #181 #182

Merged
merged 1 commit into from
Sep 4, 2023
Merged

compact implementation of ring source, close #181 #182

merged 1 commit into from
Sep 4, 2023

Conversation

fangq
Copy link
Owner

@fangq fangq commented Sep 3, 2023

@leoyala, here is my simplified implementation for the ring/ring sector source you proposed in #181. I reused most of the code in the disk source and this patch does not introduce additional registers.

I am not fully sure I understand your srcparam1.z/w settings. can you explain? in my implementation, .z/.w are simply lower and higher bound of the ring angle.

also, can you explain your srcparam2.x? mcx already uses cfg.srcdir(4) as the focal length (negative flocal length generates a diverging beam), I am wondering if srcdir(4) is sufficient to simulate the effect you desired.

@fangq fangq merged commit 287671b into master Sep 4, 2023
4 checks passed
@leoyala
Copy link

leoyala commented Sep 6, 2023

Hi @fangq, sorry for the delay in my response regarding #181.
Indeed, I agree that modeling the multiple angle ranges with multiple sources is the best way to proceed. I assume that this is already supported in MCX? Only asking because I saw #163 is still open.

An important feature of the previous implementation was the possibility to launch photons from the source according to a divergence angle, similar to the sketch below. I believe that this is not possible in the current implementation because all photons are collimated right?
The reason for the divergent beam is because of what we expect in reality to happen: sources have a numerical aperture that defines the "spread of the photons".

diverging angle

@fangq
Copy link
Owner Author

fangq commented Sep 6, 2023

An important feature of the previous implementation was the possibility to launch photons from the source according to a divergence angle, similar to the sketch below. I believe that this is not possible in the current implementation because all photons are collimated right?

this is already possible - mcx/mcxlab accepts user-defined source "focal length" for all area-based sources (disk, planar, fourier, pattern, gaussian, etc, even slit). The focal length parameter is passed to mcx via the 4th element of cfg.srcdir, a positive cfg..srcdir(4) generates a convergent beam converging at L=focal length from the source aperture; a negative cfg.srcdir(4) gives a diverging beam, with the focal plane behind the source aperture; if cfg.srcdir(4)=nan, photons are launched isotropically; if cfg.srcdir(4)=-inf, photons are launched in a lambertian profile.

in your above case, you can simply translate -R*cot(div_angle/2) as cfg.srcdir(4). do you want to try this and see if the current implementation can meet your needs?

Indeed, I agree that modeling the multiple angle ranges with multiple sources is the best way to proceed. I assume that this is already supported in MCX? Only asking because I saw #163 is still open.

you can run multiple mcx simulations and add the solution to create a composite source. implementing #163 only saves some overhead and is likely a bit faster, but the result is the same.

@leoyala
Copy link

leoyala commented Sep 7, 2023

Hi @fangq nice that this is already supported 🎉 I will give that a try 👍🏼
I am wondering if there is a roadmap for #163, this would indeed simplify our pipelines, because we would almost all the time be using multiple ring sources. But I understand if other things might take priority at the moment.

@fangq
Copy link
Owner Author

fangq commented Sep 7, 2023

Hi @fangq nice that this is already supported 🎉 I will give that a try 👍🏼

let me know if you have any question mapping your parameters to those of my implementation.

I am wondering if there is a roadmap for #163, this would indeed simplify our pipelines, because we would almost all the time be using multiple ring sources. But I understand if other things might take priority at the moment.

I wasn't giving #163 a high priority because it is essentially equivalent to running multiple sources sequentially in separate sessions and add. Because even multiple sources can be specified, the GPU will still have to randomly decide which source to use for each launched photon, which effectively serializes the simulation within each thread. For the same amount of total photon launched, the amount of the computation and GPU load is about the same as one running each source separately. The only part it saves is the memory IO cost - copy-in and copy-out of the data to and from the GPU. For shorter job, this could be significant, but for longer jobs, the overhead is relatively small.

Regardless. I still plan to work on #163 after I finish the release of v2023 that I have been preparing.

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.

None yet

2 participants