Conversation
| int Size() const; | ||
| size_t Size() const; | ||
| // Includes topological element shape | ||
| int TensorSize() const; | ||
| size_t TensorSize() const; | ||
| // Size of region that needs to be filled with 0s if not allocated | ||
| int FillSize(const IndexDomain domain) const; | ||
| size_t FillSize(const IndexDomain domain) const; |
There was a problem hiding this comment.
Those ints were the original issue.
Those propagated into the openpmd output for calculating the size of the output buffer vector and resulted in an overflow when multiplied by the number of local blocks.
Note, the hdf5 output is fine as the results from these calls are directly being put into a size_t variable.
Yurlungur
left a comment
Yurlungur
left a comment
There was a problem hiding this comment.
Thanks for the fix! This is long overdue.
I think we should use std::size_t instead of size_t just for clarity/C++ consistency.
| PARTHENON_REQUIRE_THROWS( | ||
| nbtotal < std::numeric_limits<int>::max(), | ||
| "Congratulations. You're the first one to run a simulation with more blocks than " | ||
| "max `int`. Many loops in parthenon still use `int` indices over blocks or use " | ||
| "`int` for pack indices and partitions, so who knows what happens next. Please get " | ||
| "in contact with the dev team before proceeding (or proceed on your own risk and " | ||
| "remove this statement)."); |
There was a problem hiding this comment.
Hah. Good error message.
lroberts36
left a comment
lroberts36
left a comment
There was a problem hiding this comment.
Thanks for going through and fixing all these, I know I am guilty of using int in place of size_t in many places!
| const auto rank = shape.size(); | ||
| const bool is_vector = m.IsSet(Metadata::Vector); | ||
| std::size_t total_count = count; | ||
| for (int i = 0; i < CHUNK_MAX_DIM; ++i) { | ||
| h5_offset[i] = h5_count[i] = 0; | ||
| } | ||
| for (int i = 0; i < rank; ++i) { | ||
| for (auto i = 0; i < rank; ++i) { |
There was a problem hiding this comment.
Why auto here instead of size_t?
| @@ -102,7 +102,7 @@ struct var_base_t { | |||
| KOKKOS_INLINE_FUNCTION | |||
| static int ndim() { return sizeof...(NCOMP); } | |||
There was a problem hiding this comment.
| static int ndim() { return sizeof...(NCOMP); } | |
| static size_t ndim() { return sizeof...(NCOMP); } |
3 participants
PR Summary
I ran into an issues when trying to write a conserved variable vector for a 16384^3 mesh (turns out it was more related to the local mesh size rather than the total).
This motivated going a little more through the code base and fix some type mismatches.
There are still plenty left (which should be addressed in the context of #175) .
PR Checklist