[oneTBB] Rework the parallel_for_each algorithm requirements #348

ivankochin · 2021-07-27T10:36:54Z

Now the parallel_for_each algorithm body named requirements contain unclear and incorrect formulation. This patch fixes it and makes it more clear.

Signed-off-by: Kochin, Ivan kochin.ivan@intel.com

Signed-off-by: Kochin, Ivan <kochin.ivan@intel.com>

ivankochin · 2021-07-27T10:38:00Z

@kboyarinov , @alexandraepan pls review

source/elements/oneTBB/source/named_requirements/algorithms/par_for_each_body.rst

Co-authored-by: Alexandra <alexandra.epanchinzeva@intel.com>

Signed-off-by: Kochin, Ivan <kochin.ivan@intel.com>

source/elements/oneTBB/source/named_requirements/algorithms/par_for_each_body.rst

Co-authored-by: Alexandra <alexandra.epanchinzeva@intel.com>

ivankochin · 2021-08-18T09:22:42Z

@kboyarinov, @alexandraepan please review one more time!

source/elements/oneTBB/source/named_requirements/algorithms/par_for_each_body.rst

Co-authored-by: Alexandra <alexandra.epanchinzeva@intel.com>

source/elements/oneTBB/source/named_requirements/algorithms/par_for_each_body.rst

kboyarinov · 2021-08-19T10:07:55Z

source/elements/oneTBB/source/named_requirements/algorithms/par_for_each_body.rst

+`tbb::parallel_for_each algorithm <../../algorithms/functions/parallel_for_each_func>`
+requires the Body::operator() call with an object of type const value_type& or value_type&& to be well-formed if:
+
+* the iterator does not meet all of the `Forward iterator` requirements described in the [forward.iterators] section of the ISO C++ Standard


What are we trying to say in this point? Could you please clarify the use-case?

I agree that this case seems unclear, but here we are trying to describe the case when the iterator meets input iterator requirements but does not meet forward iterator requirements.

Please, consider the following replacement:
#348 (comment)

It seems better for me, but I would swap the bullets:

... if all of the following requirements are met:

The iterator meets the requirements of Input Iterator

The iterator does not meet the requirements of Forward Iterator

As well, I would use the simple form "meets the requirements" instead of "meets all of the requirements". This "all of" is a little confusing for me

@kboyarinov, @alexandraepan please review the update

I am OK with the change

source/elements/oneTBB/source/named_requirements/algorithms/par_for_each_body.rst

Co-authored-by: kboyarinov <konstantin.boyarinov@intel.com>

ivankochin · 2021-08-20T06:26:53Z

source/elements/oneTBB/source/named_requirements/algorithms/par_for_each_body.rst

+`tbb::parallel_for_each algorithm <../../algorithms/functions/parallel_for_each_func>`
+requires the Body::operator() call with an object of type const value_type& or value_type&& to be well-formed if:
+
+* the iterator does not meet all of the `Forward iterator` requirements described in the [forward.iterators] section of the ISO C++ Standard


I agree that this case seems unclear, but here we are trying to describe the case when the iterator meets input iterator requirements but does not meet forward iterator requirements.

Please, consider the following replacement:
#348 (comment)

source/elements/oneTBB/source/named_requirements/algorithms/par_for_each_body.rst

kboyarinov · 2021-08-23T10:26:46Z

source/elements/oneTBB/source/named_requirements/algorithms/par_for_each_body.rst

+`tbb::parallel_for_each algorithm <../../algorithms/functions/parallel_for_each_func>`
+requires the Body::operator() call with an object of type const value_type& or value_type&& to be well-formed if:
+
+* the iterator does not meet all of the `Forward iterator` requirements described in the [forward.iterators] section of the ISO C++ Standard


It seems better for me, but I would swap the bullets:

... if all of the following requirements are met:

The iterator meets the requirements of Input Iterator

The iterator does not meet the requirements of Forward Iterator

As well, I would use the simple form "meets the requirements" instead of "meets all of the requirements". This "all of" is a little confusing for me

Signed-off-by: Kochin Ivan <kochin.ivan@intel.com>

source/elements/oneTBB/source/named_requirements/algorithms/par_for_each_body.rst

aepanchi · 2021-08-25T13:11:08Z

The rest looks good to me!

…r_for_each_body.rst Co-authored-by: Alexandra <alexandra.epanchinzeva@intel.com>

ivankochin · 2021-08-26T09:04:04Z

@rscohn2 please review and merge, if possible

aepanchi · 2021-09-02T13:38:28Z

@ivankochin please change .. SPDX-FileCopyrightText: 2019-2020 Intel Corporation to .. SPDX-FileCopyrightText: 2019-2021 Intel Corporation

ivankochin · 2021-09-07T11:35:46Z

@rscohn2 all comments were resolved, so please merge if possible

rscohn2 · 2021-09-07T15:40:10Z

Merged. We can add more maintainers with the ability to merge PR's if you need it.

oneTBB spec: Rework the parallel_for_each algorithm requirements

17908c3

Signed-off-by: Kochin, Ivan <kochin.ivan@intel.com>

ivankochin commented Jul 27, 2021

View reviewed changes

source/elements/oneTBB/source/named_requirements/algorithms/par_for_each_body.rst Show resolved Hide resolved

Add feeder requirements

b4702bf

aepanchi reviewed Jul 27, 2021

View reviewed changes

ivankochin and others added 3 commits July 27, 2021 15:12

Minor review improvements

0335965

Co-authored-by: Alexandra <alexandra.epanchinzeva@intel.com>

Rework the requirements by adding the dot list

f5a052d

Signed-off-by: Kochin, Ivan <kochin.ivan@intel.com>

Rephrase the iterator terd description

951b20f

Signed-off-by: Kochin, Ivan <kochin.ivan@intel.com>

ivankochin changed the title ~~oneTBB spec: Rework the parallel_for_each algorithm requirements~~ [oneTBB] Rework the parallel_for_each algorithm requirements Jul 29, 2021

Minor review updates

2fb6501

aepanchi reviewed Jul 29, 2021

View reviewed changes

Apply suggestions from code review

a101838

Co-authored-by: Alexandra <alexandra.epanchinzeva@intel.com>

aepanchi reviewed Aug 18, 2021

View reviewed changes

Apply suggestions from code review

063da8b

Co-authored-by: Alexandra <alexandra.epanchinzeva@intel.com>

aepanchi approved these changes Aug 19, 2021

View reviewed changes

kboyarinov suggested changes Aug 19, 2021

View reviewed changes

Apply suggestions from code review

093ac2c

Co-authored-by: kboyarinov <konstantin.boyarinov@intel.com>

ivankochin commented Aug 20, 2021

View reviewed changes

kboyarinov suggested changes Aug 23, 2021

View reviewed changes

Kochin Ivan added 2 commits August 25, 2021 15:43

Rephrase the requirements

df8d9a8

Signed-off-by: Kochin Ivan <kochin.ivan@intel.com>

Remove

a5e2077

Signed-off-by: Kochin Ivan <kochin.ivan@intel.com>

kboyarinov approved these changes Aug 25, 2021

View reviewed changes

Try to fix CI: add blank line after bullet list

878a5d1

Signed-off-by: Kochin Ivan <kochin.ivan@intel.com>

aepanchi reviewed Aug 25, 2021

View reviewed changes

source/elements/oneTBB/source/named_requirements/algorithms/par_for_each_body.rst Outdated Show resolved Hide resolved

Update source/elements/oneTBB/source/named_requirements/algorithms/pa…

2993afd

…r_for_each_body.rst Co-authored-by: Alexandra <alexandra.epanchinzeva@intel.com>

Update copyright year

4f9dc38

rscohn2 merged commit cfc8954 into uxlfoundation:main Sep 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[oneTBB] Rework the parallel_for_each algorithm requirements #348

[oneTBB] Rework the parallel_for_each algorithm requirements #348

ivankochin commented Jul 27, 2021

ivankochin commented Jul 27, 2021

ivankochin commented Aug 18, 2021

kboyarinov Aug 19, 2021

ivankochin Aug 20, 2021 •

edited

kboyarinov Aug 23, 2021

ivankochin Aug 25, 2021

kboyarinov Aug 25, 2021

ivankochin Aug 20, 2021 •

edited

kboyarinov Aug 23, 2021

aepanchi commented Aug 25, 2021

ivankochin commented Aug 26, 2021

aepanchi commented Sep 2, 2021

ivankochin commented Sep 7, 2021

rscohn2 commented Sep 7, 2021

[oneTBB] Rework the parallel_for_each algorithm requirements #348

[oneTBB] Rework the parallel_for_each algorithm requirements #348

Conversation

ivankochin commented Jul 27, 2021

ivankochin commented Jul 27, 2021

ivankochin commented Aug 18, 2021

kboyarinov Aug 19, 2021

Choose a reason for hiding this comment

ivankochin Aug 20, 2021 • edited

Choose a reason for hiding this comment

kboyarinov Aug 23, 2021

Choose a reason for hiding this comment

ivankochin Aug 25, 2021

Choose a reason for hiding this comment

kboyarinov Aug 25, 2021

Choose a reason for hiding this comment

ivankochin Aug 20, 2021 • edited

Choose a reason for hiding this comment

kboyarinov Aug 23, 2021

Choose a reason for hiding this comment

aepanchi commented Aug 25, 2021

ivankochin commented Aug 26, 2021

aepanchi commented Sep 2, 2021

ivankochin commented Sep 7, 2021

rscohn2 commented Sep 7, 2021

ivankochin Aug 20, 2021 •

edited

ivankochin Aug 20, 2021 •

edited