move CellDataTransferBuffer out of internal namespace #5384
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/run-tests |
bangerth
reviewed
Nov 3, 2017
| /** | ||
| * A structure that allows the transfer of data of type T from one processor | ||
| * to another. It corresponds to a packed buffer that stores a list of | ||
| * cells and an array of type T. |
There was a problem hiding this comment.
This makes it sound like we use different containers for the cells and for the Ts, but we really just use std::vector for both. Please make sure that you use the same noun for both.
| * to another. It corresponds to a packed buffer that stores a list of | ||
| * cells and an array of type T. | ||
| * | ||
| * The vector @p data is the same size as @p cell_ids. |
There was a problem hiding this comment.
You state it as a fact, but it's really a requirement that the user has to satisfy. Can you move this to the documentation of save() and mark it with @pre as a precondition for this function?
| * The vector @p data is the same size as @p cell_ids. | ||
| */ | ||
| template <int dim, typename T> | ||
| struct CellDataTransferBuffer |
There was a problem hiding this comment.
-
I think it would be useful if the class doc would say that the class facilitates this transfer by providing the
save/loadfunctions that are able to pack everything up into a stream. -
I think it would also be useful to state the requirements on
T, namely that it can be serialized via theboost::serializationframework.
davydden
force-pushed
the
cell_data_transfer_for_users
branch
from
faa3c64
to
5cb8599
Compare
|
@bangerth i update the documentation, let me know if you have further comments. |
davydden
force-pushed
the
cell_data_transfer_for_users
branch
2 times, most recently
from
11e052f
to
04b07ae
Compare
bangerth
reviewed
Nov 3, 2017
| struct CellDataTransferBuffer | ||
| { | ||
| /** | ||
| * vector to store IDs of cells to be transfered. |
There was a problem hiding this comment.
Capitalize v here. Same for the next function
There was a problem hiding this comment.
Vector creates a hyperlink to dealii's vector. I will write Standard vector if you don't mind
There was a problem hiding this comment.
Yes, that works. Or "A vector to store...", which is actually better grammar anyway.
| * CellId and a vector of type @p T. | ||
| * | ||
| * This class facilitates the transfer by providing the save/load functions | ||
| * that are able to pack up the vector of CellId 's and the associated |
davydden
force-pushed
the
cell_data_transfer_for_users
branch
from
04b07ae
to
8283730
Compare
davydden
force-pushed
the
cell_data_transfer_for_users
branch
from
8283730
to
00c1e49
Compare
|
I think that if you write it as "%Vector” then it should not generate a link.
https://www.stack.nl/~dimitri/doxygen/manual/autolink.html#linkclass <https://www.stack.nl/~dimitri/doxygen/manual/autolink.html#linkclass>
… On 04 Nov 2017, at 09:09, Denis Davydov ***@***.***> wrote:
@davydden commented on this pull request.
In include/deal.II/distributed/grid_tools.h <#5384 (comment)>:
> + * A structure that allows the transfer of cell data of type @p T from one processor
+ * to another. It corresponds to a packed buffer that stores a vector of
+ * CellId and a vector of type @p T.
+ *
+ * This class facilitates the transfer by providing the save/load functions
+ * that are able to pack up the vector of CellId 's and the associated
+ * data of type @p T into a stream.
+ *
+ * Type @p T is assumed to be serializable by <code>boost::serialization</code> (for
+ * example <code>unsigned int</code> or <code>std::vector@<double@></code>).
+ */
+ template <int dim, typename T>
+ struct CellDataTransferBuffer
+ {
+ /**
+ * vector to store IDs of cells to be transfered.
Vector creates a hyperlink to dealii's vector. I will write Standard vector if you don't mind
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub <#5384 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJ-dwpQ4y0fkyfoDiMDSfODT5dkcJxWBks5szBuvgaJpZM4QRV6K>.
|
|
Thanks, will fix
Sent from iPhone, please excuse my brevity.
… 4 нояб. 2017 г., в 10:59, Jean-Paul Pelteret ***@***.***> написал(а):
I think that if you write it as "%Vector” then it should not generate a link.
https://www.stack.nl/~dimitri/doxygen/manual/autolink.html#linkclass <https://www.stack.nl/~dimitri/doxygen/manual/autolink.html#linkclass>
> On 04 Nov 2017, at 09:09, Denis Davydov ***@***.***> wrote:
>
> @davydden commented on this pull request.
>
> In include/deal.II/distributed/grid_tools.h <#5384 (comment)>:
>
> > + * A structure that allows the transfer of cell data of type @p T from one processor
> + * to another. It corresponds to a packed buffer that stores a vector of
> + * CellId and a vector of type @p T.
> + *
> + * This class facilitates the transfer by providing the save/load functions
> + * that are able to pack up the vector of CellId 's and the associated
> + * data of type @p T into a stream.
> + *
> + * Type @p T is assumed to be serializable by <code>boost::serialization</code> (for
> + * example <code>unsigned int</code> or <code>std::vector@<double@></code>).
> + */
> + template <int dim, typename T>
> + struct CellDataTransferBuffer
> + {
> + /**
> + * vector to store IDs of cells to be transfered.
> Vector creates a hyperlink to dealii's vector. I will write Standard vector if you don't mind
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub <#5384 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJ-dwpQ4y0fkyfoDiMDSfODT5dkcJxWBks5szBuvgaJpZM4QRV6K>.
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
davydden
force-pushed
the
cell_data_transfer_for_users
branch
from
00c1e49
to
6ebc906
Compare
davydden
force-pushed
the
cell_data_transfer_for_users
branch
from
6ebc906
to
83625d7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
this class is very useful in user projects. So i think it should be in
parallel::GridTools::namespace and documented.p.s. no functional changes in the first commit, the rest is minor/cosmetic.