Diagonal tree balance

[jcapriot]

This adds support to build tree meshes that are balanced along diagonals in addition to along axis directions.

This is equivalent to the types of meshes that the UBC-GIF codes create.

Without diagonal balancing:

With diagonal balancing:

[codecov]

Codecov Report

Merging #295 (838f23e) into main (c0894e6) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #295      +/-   ##
==========================================
+ Coverage   84.36%   84.41%   +0.04%     
==========================================
  Files          36       36              
  Lines       11001    11014      +13     
==========================================
+ Hits         9281     9297      +16     
+ Misses       1720     1717       -3     

Impacted Files	Coverage Δ
discretize/_extensions/tree_ext.pyx	81.87% <100.00%> (+0.07%)	⬆️
discretize/tree_mesh.py	61.19% <100.00%> (+0.94%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[jcapriot]

It is currently implemented as a property that can only be toggled during tree mesh creation, to keep the whole mesh either one way or the other. The other option (which can be switched pretty quickly here) is to allow it to be toggled at each call to the refine functions.

i.e.
With this implementation it must be set on instantiation:
mesh = TreeMesh([hx, hy, hz], [0, 0, 0] diagonal_balance=True)

But with not much work it could also be toggled more fine-grained on the calls to the individual refine functions:
mesh.refine_box(x0, xF, level, diagonal_balance=True)

Both ways work, just the question is which is more intuitive. I could also enable an option to control balancing along the cardinal directions as well...

[domfournier]

Hmm not sure, but sort of more intuitive to me to be at the top level and enforced for all refinements. Feels like it should be defaulted to True for sure. The non-balanced version is cheaper but less well-behaved.

[jcapriot]

I feel like defaulting to true might have a few more implications than you might expected. While in general it is a good thing to balance across diagonal cells, we need to be very careful about how this is handled.

Essentially it would mean that every older mesh that might be read in would be automatically diagonally balanced. (i.e. you read in an older TreeMesh, with which you want to load in the corresponding model with it.) The way the read-ins work, is that it essentially inserts every cell of the old mesh, into the new mesh using the insert_cells call.

Maybe the answer is to be very upfront about changing it in the future, keep the default as false, but issue a warning about the default behavoir changing in a future release...

[jcapriot]

@domfournier I just pushed a small change in the last commit here that should allow us to support older (non-diagonally balanced) mesh read in when we do change the default behavoir in the future.

Essentially the answer is to never enforce diagonal balancing when we are explicitly setting the state of the mesh. The logic goes:

• The information describing the state of the mesh lists every cell in the mesh.
• If it was diagonally balanced on creation, those cells would naturally be included in the state.
• Therefore, no need to explicitly enforce diagonal balancing when setting the state.

I think in the next release I'll add a warning message to the user saying that the default behavior will be changing.

[domfournier]

Sounds great.

[domfournier]

Can't really comment on the C code, but looks good. Ready for testing.