Small: Quality of life changes for downstream codes #1143
Conversation
src/interface/variable_state.hpp
Outdated
| int t{0}; | ||
| int u{0}; | ||
| int v{0}; |
There was a problem hiding this comment.
Maybe should be initialized to some signal var indicating they haven't been set, like -1?
There was a problem hiding this comment.
👍 on the default init.
Unrelated to this PR and a more general performance question: Has anyone looked at the impact on performance when putting more and more info in the packs?
There was a problem hiding this comment.
yeah, probably so. Also maybe would be better to store as an array? Not totally sure.
There was a problem hiding this comment.
The message from @pgrete didn't show up for me before. I have not looked carefully at the performance impact, but at least when I put variable state in initially I didn't see any slow down. Of course, there is now more state stored there...
There was a problem hiding this comment.
If this is a performance issue, there are definitely things that can be removed and some things that can be moved to the sparse pack itself. That being said, having easy access to this information simplifies a lot of code.
src/interface/variable_state.hpp
Outdated
| int t{0}; | ||
| int u{0}; | ||
| int v{0}; |
There was a problem hiding this comment.
👍 on the default init.
Unrelated to this PR and a more general performance question: Has anyone looked at the impact on performance when putting more and more info in the packs?
lroberts36
changed the title
Co-authored-by: Adam C Reyes <[email protected]>
…arthenon into lroberts36/add-small-riot
| std::vector<int> shape = {}) | ||
| : params_(params_in), iter_counter(0), eqs_(eq_in) { | ||
| std::vector<int> shape = {}, const std::string &container = "base") | ||
| : params_(params_in), iter_counter(0), eqs_(eq_in), container_(container) { |
There was a problem hiding this comment.
Just thinking out loud here (independent of this PR): We are still sharing comm buffer of each variable across containers, aren't we?
Do we have any control over ensuring that only one communication piece is running/using the comm buffers in a task list at any given time?
There was a problem hiding this comment.
We don't have any control over that and I agree that if you had a task list that had independent groups of tasks that worked on different containers but also communicated you could possibly run into issues where one container grabs messages meant for another container. That is similar to some of the issues I had to fix for multi-grid in this PR. I am not sure what the best approach to dealing with this is. Making buffers for each container doesn't seem ideal and it could be a little tricky to build buffers and MPI tag maps for multiple containers in the current framework (since now buffers are built right after remesh, not at the point when containers are first asked for). Maybe we should put up a warning issue about this.
PR Summary
ParArray3Din pack usingVariableState.PhysicalConstantsTypeListwrapper for lists of types that allows for concatenation, pulling out types at specific indices, etc.PR Checklist