Closed pedrolimasi closed 11 months ago
Skonki
commented
11 months ago @pedrolimasi Let me get familiar with reviewing the pull request. However, I am not fine with changing this back to always loading all meshes....
The reason is simple, if you have large meshes and many time-steps. You unnecessarily fill up your ram such that in several post-processing steps for buoyant fractures it is not viable anymore.
pedrolimasi
commented
11 months ago @Skonki: [...] The reason is simple, if you have large meshes and many time-steps. You unnecessarily fill up your ram such that in several post-processing steps for buoyant fractures it is not viable anymore.
Interesting. I would've expected these mesh-loading operations to move around a pointer/reference to a single CartesianMesh instance (i.e. no mesh copy operations consuming memory).
Maybe we can find a simple design to force this to be the case?
If you're working on this right now, would you like to jump into a zoom call with me for us to team up on this?
Skonki
commented
11 months ago @pedrolimasi Well theoretically today is my day off so...^^
But we can check it tomorrow together. I guess the fact that all are individual instances comes from the fact that we safe them as such (which makes sense). Then in the loading we simply load the instance with when we load the dill. I changed it to work on an integer, we could try to switch this to have it on a pointer instead. As I agree, this would be the more pythonic way of doing it.
Skonki
commented
11 months ago Closed because we resolved it with a different pull request (#19 )
BugFix for confusion on mesh property of Fracture instances. This commit reverts
@Skonki, I'm reasonably confident this PR should be merged as is. But, since I'm reverting a fair chunk of PostProcess code you wrote 11months ago, you may want to take a look.
Issue #12 gives more context to this change.