-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Add tutorial for creating custom executors #4359
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: master
Are you sure you want to change the base?
Add tutorial for creating custom executors #4359
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please format the cpp file (eg: std::cout <<
notice the added spaces between different literals)
Also, CMakeLists.txt is missing
doc/tutorials/content/sources/custom_executor/custom_executor.cpp
Outdated
Show resolved
Hide resolved
doc/tutorials/content/sources/custom_executor/custom_executor.cpp
Outdated
Show resolved
Hide resolved
|
||
.. note:: | ||
This is an advanced topic and requires some knowledge on working of executors | ||
and its implementation in PCL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any location at the moment to where we can point the reader for additional info?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea was to link them to the design document and Code API.
Both are yet to be merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create an issue to link them (in your fork or PCL) so we don't forget
doc/tutorials/content/sources/custom_executor/custom_executor.cpp
Outdated
Show resolved
Hide resolved
doc/tutorials/content/sources/custom_executor/custom_executor.cpp
Outdated
Show resolved
Hide resolved
doc/tutorials/content/sources/custom_executor/custom_executor.cpp
Outdated
Show resolved
Hide resolved
doc/tutorials/content/sources/custom_executor/custom_executor.cpp
Outdated
Show resolved
Hide resolved
// Forward declaration for custom executor | ||
template <typename Blocking, typename ProtoAllocator> | ||
struct omp_benchmark_executor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this forward declaration required? What happens if you move the is_executor_available
after the class is defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is needed since we check for this inside the class:
static_assert(is_executor_available_v<omp_executor>, "OpenMP benchmark executor unavailable");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That static assert checks for omp_executor
, not omp_benchmark_executor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops. Fixed now.
|
||
.. note:: | ||
This is an advanced topic and requires some knowledge on working of executors | ||
and its implementation in PCL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create an issue to link them (in your fork or PCL) so we don't forget
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Tutorial and code are simple enough :)
again. Do not merge until executors are merged into the main repo |
No description provided.