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

Source sampling #478

Merged
merged 4 commits into from
Mar 18, 2024
Merged

Source sampling #478

merged 4 commits into from
Mar 18, 2024

Conversation

JulienDoerner
Copy link
Member

The sampling of the position in the SNR and pulsar distribution is done in cylindrical coordinates. The current approach misses a $r$ term for the radial componet. The differential number of sampled positions should be given by $dN = f(R, z) dV = f(R, z) \ R dR\ dZ \ d\phi$.

Additionaly the PR reduces the amount computation by skipping the normalisation of the curves. For the MC-rejection sampling only the knowleadge of the overall maximum is needed (once calculated) and not a normalized curve.

src/Source.cpp Outdated Show resolved Hide resolved
@lukasmerten
Copy link
Member

Looks generally good to me. I agree that we missed that additional factor from the volume element before.

@lukasmerten
Copy link
Member

Thanks, @JulienDoerner. Let's for some more comments and merge otherwise next week.

@rafaelab
Copy link
Member

Looks good to me, @JulienDoerner .

@lukasmerten lukasmerten merged commit d6a4670 into CRPropa:master Mar 18, 2024
4 checks passed
@JulienDoerner JulienDoerner deleted the source_sampling branch March 19, 2024 07:21
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.

3 participants