parthenon
parthenon copied to clipboard
`Remove` methods
Both MeshBlockData
and SwarmContainer
have Remove
methods, which allow one to remove a field (either a variable or a particle). This strikes me as kind of counter to the parthenon model, where we describe what we want at the state descriptor level and then don't change things. I also don't think it's being used, so I'd like to elliminate it. Is anyone using these methods and would they object to removing them?
Pinging @pgrete and @lroberts36 . And especially @bprather because if anyone is ~~abusing~~ relying on this it's you. Already spoke to @brryan and he's on board.
I am abusing this, actually! How did you guess?
The central problem this solves for me is I want a single simulation with two distinct phases:
- Clean up magnetic field with an elliptic solve
- Evolve fluid normally
Both require using Parthenon's MPI syncs, but need to sync different variables. Rather than mess with metadata after the fact, I just remove the variables I don't need from the container I'm using.
The appropriate solution would be to allow syncing sub-sets of variables on any given MPI call -- but for my particular case the sub-sets machinery would need to further be threaded through the solvers, so it would be quite a process.
@bprather one reason I bring this up is I think the new flags machinery may not entirely clean up after itself. In particular I don't think the MeshblockData::flagsToVars_
object is cleaned up when remove
is called.
As an aside, why can't you make a new container containing fewer variables?
Hmm, maybe CopyFrom
is all I need. Certainly for splitting off the solver step, I should be using that instead of copying and removing.
I think the issue is that by default the "base" block has all variables that were added in package initialization, I think. So I need to remove something from it at some point.
I am a little confused, it looks like MeshBlockData::Remove
is defined as
template <typename T>
void MeshBlockData<T>::Remove(const std::string &label) {
throw std::runtime_error("MeshBlockData<T>::Remove not yet implemented");
}
on develop
. Given this, I would suggest removing it.
@bprather did you implement this functionality on another branch?
You know, looking back I did "implement" it in my branch for KHARMA, and then just forgot. It's just one line, varMap_.erase(label);
...meaning it probably plays hell with memory, but I only needed to remove the MPI sync calls, which appeared to be enumerated from the varMap...
Anyway yes, I'm now definitely in favor of removing it entirely. I'll deal with switching to CopyFrom
when I next pull develop.
Sounds good. I'll remove these then when I do some other plumbing.