Improve ScatterView Documentation #212
Conversation
ajpowelsnl
changed the title
| friend class ScatterAccess<DataType, Op, ExecSpace, Layout, {ScatterNonDuplicated,ScatterDuplicated}, contribution, ScatterAtomic>; | ||
| friend class ScatterAccess<DataType, Operation, ExecSpace, Layout, {ScatterNonDuplicated,ScatterDuplicated}, contribution, ScatterNonAtomic>; | ||
| friend class ScatterAccess<DataType, Operation, ExecSpace, Layout, {ScatterNonDuplicated,ScatterDuplicated}, contribution, ScatterAtomic>; | ||
| []:# (TODO: Deprecate types requiring `Kokkos::Impl..` ) |
There was a problem hiding this comment.
Did you mean to commit this line?
There was a problem hiding this comment.
Do you mean the comment line, []:# (TODO: Deprecate types requiring Kokkos::Impl.. )? If yes, that was my initial intent, but I can remove it.
There was a problem hiding this comment.
No I mean "commit". Are you sure about the syntax? The line did show on the rendered diff
|
|
||
| template <typename... P, typename... Dims> | ||
| ScatterView(::Kokkos::Impl::ViewCtorProp<P...> const& arg_prop, Dims... dims); | ||
| []: # (TODO: Deprecate types requiring `Kokkos::Impl..` ) |
There was a problem hiding this comment.
Another comment line. I will remove it.
| @@ -23,53 +28,53 @@ Kokkos::Experimental::contribute(results, scatter); | |||
| ```c++ | |||
|
|
|||
| template <typename DataType | |||
| ,int Op | |||
| ,int Operation | |||
| ,typename ExecSpace | |||
| ,typename Layout | |||
| ,int contribution | |||
| > | |||
| class ScatterView<DataType | |||
There was a problem hiding this comment.
remove all the template arguments blower here i.e. lets just make this:
template<class DataType, class Layout, class ExecSpace, class Operation, class Duplication, class Contribution>
class ScatterView {And then use Contribution instead of {ScatterNonDuplicated, ScatterDuplicated} that just didn't make any sense to write it the way it was.
Then you need to introduce a Parameters section like in Kokkos::View, before describing the typedefs.
In the parameters section you describe what the parameters are.
There was a problem hiding this comment.
@crtrott - apologies for the delayed update. I'm not sure what the class Duplication parameter specifies -- would potential values for this parameter be ScatterNonDuplicated, ScatterDuplicated? In any case, I've cleaned the entry up, and tried to get it into a basically correct and current state.
ajpowelsnl
force-pushed
the
documentation/ScatterView
branch
from
b748aeb
to
5769809
Compare
| .. rubric:: Public Member Variables | ||
| * ``Layout``: See `Kokkos View LayoutType <../core/view/view.html>`_ | ||
|
|
||
| * ``ExecutionSpace``: Where the code will be executed (CPU or GPU); typical values are ``Kokkos::DefaultHostExecutionSpace``, ``Kokkos::DefaultExecutionSpace`` |
There was a problem hiding this comment.
Instead of listing typical values, I would just mention that this defaults to Kokkos::DefaultExecutionSpace if not specified.
| @@ -96,18 +91,36 @@ Description | |||
| resize a view with discarding old data | |||
|
|
|||
|
|
|||
| .. rubric:: *Private* Members | |||
| Constructors | |||
There was a problem hiding this comment.
I think this constructor subsection should be placed before the public class methods subsection.
ajpowelsnl
force-pushed
the
documentation/ScatterView
branch
from
07a3563
to
a281829
Compare
This PR aims to improve readability by spelling out concepts represented by terse, non-standard acronyms (e.g., RT, RP) and linking to
Viewdocumentation. Additionally, a brief usage explanation is provided, as are deprecationTODOaroundKokkos::Implstatements.