-
Notifications
You must be signed in to change notification settings - Fork 463
Adding support to memory collection of Particles and MultiFabs within a grid #4629
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
Changes from 1 commit
7a29e92
20d8658
b615a4e
db786f3
1bbf3ca
5f0f788
a390353
7fb20d4
fb94457
6c0e6e2
6b231b7
bad61d0
8034c87
2d2ddd0
6cf35fa
327a241
6c0d68e
58114a0
84bbf37
55d4a34
64e1e84
2b01097
c399f60
63b21a5
11dd878
30ec777
d648a26
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -817,6 +817,10 @@ public: | |
| template <class F=FAB, std::enable_if_t<IsBaseFab<F>::value,int> = 0> | ||
| void setDomainBndry (value_type val, int strt_comp, int ncomp, const Geometry& geom); | ||
|
|
||
| // Get capacity of the FabArray | ||
| template <class F=FAB, std::enable_if_t<IsBaseFab<F>::value,int> = 0> | ||
| void capacityOfFabs (LayoutData<std::size_t>& mem); | ||
|
jessdagostini marked this conversation as resolved.
Outdated
|
||
|
|
||
| /** | ||
| * \brief Returns the sum of component "comp" | ||
| * | ||
|
|
@@ -2506,6 +2510,16 @@ FabArray<FAB>::setDomainBndry (value_type val, | |
| } | ||
| } | ||
|
|
||
| template <class FAB> | ||
| template <class F, std::enable_if_t<IsBaseFab<F>::value,int> FOO> | ||
| void | ||
| FabArray<FAB>::capacityOfFabs(LayoutData<std::size_t>& mem) { | ||
|
jessdagostini marked this conversation as resolved.
Outdated
|
||
| // Should we have assertions here? What should be asserted? | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be good to assert that
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it! Just pushed the assertion on 7a29e92 |
||
| for (MFIter mfi(*this, MFItInfo{}.DisableDeviceSync()); mfi.isValid(); ++mfi) { | ||
| mem[mfi] += (*this)[mfi].nBytesOwned(); | ||
| } | ||
| } | ||
|
|
||
| template <class FAB> | ||
| template <class F, std::enable_if_t<IsBaseFab<F>::value,int> FOO> | ||
| typename F::value_type | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
For flexibility, we could also make the integer type a template typename here and for in particle containers. For example,
And in the implementation, we also want to do explicit static_cast before adding the values.
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.
Ok! Working on the update for this!
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.
Actually, do you think it's worth doing it? Because I know that it is a best practice in C++ to use
std::size_tfor memory-related math (size_twas created for holdingsizeofreturns https://en.cppreference.com/w/cpp/types/size_t.html)Uh oh!
There was an error while loading. Please reload this page.
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.
Actually the best practice is proably to avoid unsigned integers in almost all cases.
Even Stroustrup does not like unsigned integers. https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1428r0.pdf Stroustrup said,
Java does not even have build-in unsigned integer types. https://www.artima.com/articles/james-gosling-on-java-may-2001 Goshling said,
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.
Hmm, got it! Interesting that the unsigned is such unknown territory. Will start working on including the template you suggested in both functions
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.
Just update both function signatures with the new template 55d4a34