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

change IsInside check for UnionAll to act only on the selected solid not the full collection. #212

Merged
merged 3 commits into from
Dec 29, 2024

Conversation

tdixon97
Copy link
Collaborator

@tdixon97 tdixon97 commented Dec 29, 2024

Fixes #209 (at least for UnionAll), however, maybe we would like a cleaner solution than making a container with only 1 solid in it.
We also need to:

@gipert
Copy link
Member

gipert commented Dec 29, 2024

Yes please, can you implement SampleableObject::IsInside() and update SampleableObjectCollection::IsInside() to use it?

bool RMGVertexConfinement::SampleableObjectCollection::IsInside(const G4ThreeVector& vertex) const {
auto navigator = G4TransportationManager::GetTransportationManager()->GetNavigatorForTracking();
for (const auto& o : data) {
if (o.physical_volume) {
if (navigator->LocateGlobalPointAndSetup(vertex) == o.physical_volume) return true;
} else {
if (o.sampling_solid->Inside(o.rotation.inverse() * vertex - o.translation)) return true;
}
}
return false;
}

@tdixon97
Copy link
Collaborator Author

Yeah I thought about it, I can definitely do this if we prefer that solution.

@gipert
Copy link
Member

gipert commented Dec 29, 2024

I want to setup Python for tests today, with an example, after #210 is merged.

@tdixon97
Copy link
Collaborator Author

Ok lets try to implement the tests later then, I have anyway all the python scripts I used.

@tdixon97
Copy link
Collaborator Author

Have a look now @gipert

@gipert
Copy link
Member

gipert commented Dec 29, 2024

Can you also update SampleableObjectCollection::IsInside() to use it?

@tdixon97
Copy link
Collaborator Author

tdixon97 commented Dec 29, 2024 via email

@gipert
Copy link
Member

gipert commented Dec 29, 2024

Did it no worries, I merge this and you can add/fix the rest in a future PR.

@gipert gipert added the bug Something isn't working label Dec 29, 2024
@gipert gipert merged commit 3dd14e4 into legend-exp:main Dec 29, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible bug in sampling from multiple sources
2 participants