search

lanl / phoebus

Phifty One Ergs Blows Up A Star
BSD 3-Clause "New" or "Revised" License
46 stars 2 forks source link

reduce geometry memory footprint #101

Closed Yurlungur closed 2 years ago

Yurlungur commented 2 years ago

PR Summary

This is based on a discussion with @jdolence about what geometry objects we actually need to access and from where. The geometry cache, especially the metric derivatives, take up a lot of memory. I am currently storing all geometry objects at all cell centers, faces, and nodes. However, we only really need metric derivatives at cell centers (for source terms) and the nodes are totally untouched.

This PR removes node-centered variables entirely, except for "physical coordinate locations," which is useful for 3D visualization tools like visit. It also removes metric derivatives from cell faces. This reduces the memory footprint of the caching machinery substantially... by a bit more than a factor of 2.

The price, of course, is that metric derivatives---which are needed to compute the christoffel symbols---are no longer available at faces and nodes. I don't believe we need them there, but I could be wrong. In particular, @brryan do we need the christoffel symbols in the ghost zones? If so, face- or node-centered data is probably useful for interpolation. If not, though, we can probably get away with these savings.

Another price is to be paid in the API for the geometry. I now have the code die (in debug mode) if you try to access node-centered data where it is not available. Is this an acceptable price?

I'm interested in folk's thoughts. If people like this, then we should run a torus, and the snake linear modes test, with caching on to make sure I didn't break anything.

PR Checklist

Yurlungur commented 2 years ago

Running on the torus revealed a mistake on my end with the Coords array in CachedSystem. I do need to set in CachedSystem as SetGeometryDefault was using the values in the cache.

It also revealed two long-standing bugs:

  1. The Coords function in ModifiedSystem was incorrectly returning the coordinate transformation vector, not the Cartesian coordinates for visualization.
  2. SetCachedGeometry and SetGeometryDefault were both filling the Coords array, duplicating both code and work. To fix this this, I make SetCachedGeometry call SetGeometryDefault and have SetGeometryBlock only call SetGeometryDefault if the Coords array actually needs to be set.