-
Notifications
You must be signed in to change notification settings - Fork 6
664 missing assertions for threaded execution #284
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
base: develop
Are you sure you want to change the base?
Conversation
|
Ready to merge |
48a6c1a to
775ca77
Compare
|
@anyzelman this seems solved, I don't know if we want a clearer error message to be outputted instead of a possibly cryptic assertion error. |
775ca77 to
11b57b6
Compare
|
I rebased again, reviewed it, and running smoke + unit tests with LPF now (in addition to CI). Changes during review: I made the check a function of the Coordinates class, as otherwise it is not easy to call for a check without a lot of code repetition. I also modified the internal::getCoordinates for const-coordinates to perform the check. I did not think the modification to config::OMP was needed and reverted that, instead relying on its already-established threads function call. There is one annoyance that in the use of the coordinates class, a callee tends to call a third class for creating the buffer, and then passes that into Coordinates. This means that the number of threads is actually controlled by this third class. The current design in this MR is hence not totally robust if somehow the number of threads spawned during a parallel section changes between the time the third party class code executes and allocates the buffer for some T1 #threads , and the time the #threads is actually recorded when the coordinates instance is set while some other T2 #threads is active. |
… sequential and parallel contexts. Refined it a bit and trying again
Closes #278