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

Cleanup the Vector::thread_loop_partitioner logic. #7683

Merged

Conversation

drwells
Copy link
Member

@drwells drwells commented Feb 4, 2019

It was previously possible to, in one function call, either set up the partitioner multiple times or for inappropriately small vectors. This commit cleans up the way we handle the partitioner in multiple places so that, when possible, vectors share partitioners and do not set up partitioners if they are too small.

Fixes #7646.

@drwells
Copy link
Member Author

drwells commented Feb 4, 2019

/rebuild

@drwells drwells force-pushed the cleanup-vector-thread-partitioner branch from 715ecb4 to 5fe0ed6 Compare February 4, 2019 04:20
Copy link
Member

@kronbichler kronbichler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Is there anything in la_parallel_vector.h that can/should be treated similarly?

@masterleinad
Copy link
Member

We have a number of tests failing

4207 - multigrid/transfer_block_select.debug (Failed)
4220 - multigrid/transfer_prebuilt_02.debug (Failed)
4221 - multigrid/transfer_system_05.debug (Failed)
4223 - multigrid/transfer_01.debug (Failed)
4231 - multigrid/transfer_system_adaptive_07.debug (Failed)
4235 - multigrid/transfer_compare_01.debug (Failed)
4245 - multigrid/transfer_03.debug (Failed)
4256 - multigrid/transfer_block.debug (Failed)
4259 - multigrid/transfer_system_adaptive_08.debug (Failed)
4261 - multigrid/transfer_system_04.debug (Failed)
4267 - multigrid/transfer_system_adaptive_01.debug (Failed)
4272 - multigrid/transfer_system_adaptive_03.debug (Failed)
4274 - multigrid/transfer_system_02.debug (Failed)
4277 - multigrid/transfer_system_adaptive_09.debug (Failed)
4280 - multigrid/transfer_prebuilt_01.debug (Failed)
4283 - multigrid/transfer_system_adaptive_04.debug (Failed)
4288 - multigrid/transfer_system_03.debug (Failed)
4289 - multigrid/transfer_system_adaptive_02.debug (Failed)
4294 - multigrid/transfer_prebuilt_03.debug (Failed)
4299 - multigrid/transfer_02.debug (Failed)
4305 - multigrid/transfer_select.debug (Failed)
4306 - multigrid/transfer_system_01.debug (Failed)

with the error

multigrid/transfer_system_01.debug: RUN failed. ------ Additional output on stdout/stderr:


--------------------------------------------------------
An error occurred in line <186> of file </jenkins/workspace/dealii_PR-7683/include/deal.II/lac/vector_operations_internal.h> in function
    void dealii::internal::VectorOperations::parallel_for(Functor&, dealii::internal::VectorOperations::size_type, dealii::internal::VectorOperations::size_type, const std::shared_ptr<dealii::parallel::internal::TBBPartitioner>&) [with Functor = dealii::internal::VectorOperations::Vector_set<double>; dealii::internal::VectorOperations::size_type = unsigned int]
The violated condition was: 
    partitioner.get() != nullptr
Additional information: 
    Unexpected initialization of Vector that does not set the TBB partitioner to a usable state.
--------------------------------------------------------

@drwells
Copy link
Member Author

drwells commented Feb 4, 2019

All of those tests pass on my laptop :( I am investigating.

@drwells
Copy link
Member Author

drwells commented Feb 4, 2019

@kronbichler I think that vector class is in better shape (it swaps partitioners correctly). I will take a closer look later.

@drwells drwells force-pushed the cleanup-vector-thread-partitioner branch from 5fe0ed6 to 518cdef Compare February 5, 2019 00:29
@drwells
Copy link
Member Author

drwells commented Feb 5, 2019

I think I fixed the issues.

It was previously possible to, in one function call, either set up the
partitioner multiple times or for inappropriately small vectors. This
commit cleans up the way we handle the partitioner in multiple places so
that, when possible, vectors share partitioners and do not set up
partitioners if they are too small.
@drwells drwells force-pushed the cleanup-vector-thread-partitioner branch from 518cdef to 461fb77 Compare February 5, 2019 00:31
@masterleinad
Copy link
Member

This passes the test suite for me locally. Let's merge.

@masterleinad masterleinad merged commit 5d4a719 into dealii:master Feb 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants