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

Fix822 ffp early stop with open mp #823

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

NikEfth
Copy link
Collaborator

@NikEfth NikEfth commented Feb 18, 2021

This is a small patch to fix early stop when forward projecting with OpenMP.
The issue is not 100% reproducible but I have experienced many times.

@NikEfth
Copy link
Collaborator Author

NikEfth commented Feb 18, 2021

@KrisThielemans Please wait before you review this. It will fail without OpenMP, I forgot to fix that.

@KrisThielemans
Copy link
Collaborator

I don't think we've ever seen this problem, @AnderBiguri and @danieldeidda can probably comment as they both have "monster" machines. But obviously that doesn't mean that it doesn't exist (wtih multi-threading, you never know).

However, I cannot see why this modification would help. In the original code, the viewgrams are a local variable, so they are thread-specific already.

@NikEfth
Copy link
Collaborator Author

NikEfth commented Feb 18, 2021

I agree, that viewgrams is local and there should not be a difference.
As you know for the normalisation code I had to forward project a model of the source used in the simulation.
Many times, the projection was finishing without reported problems, but a few megabytes short.
With the suggested modification I have not faced the problem again.

@NikEfth
Copy link
Collaborator Author

NikEfth commented Feb 18, 2021

P.S.
Maybe is an OPENMP version, specific problem. I am using Arch Linux, which means I have the latest version.

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