Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tree mesh acceleration #94

Merged
merged 71 commits into from
Jul 20, 2018
Merged

Tree mesh acceleration #94

merged 71 commits into from
Jul 20, 2018

Conversation

jcapriot
Copy link
Member

@jcapriot jcapriot commented May 2, 2018

At this point consider this branch as EXPERIMENTAL. There are many possible unsafe operations that could arise, so be careful (which will need to be enforced at a later time). The code is certainly not completely up to format standards, but at this point, just wanted to get a pull request going and allow anyone who wants to use it to test it out to find any bugs.

There are many changes to the TreeMesh implementation within this pull request, However it is mostly feature complete compared to the previous implementation with regards to the public members of the class.

It was mostly rewritten in a way that made the construction of the mesh and construction of the operators all done in c++/cython, which resulted in dramatic speedups. As a reference, the 97 nosetests on the tree in the previous implementation took 238.476s on my personal computer. This implementation took 20.281s.

The basic idea is that each object, (node, edge, etc.) is aware of the structure of the TreeMesh as each is a cpp class that contains references to other objects.

A few key differences:

  • All tree construction is balanced (no need to call tree.balance, or pass balance=True to functions)

  • tree.refine should only be called once (at this point) as it "finalizes" the tree. It might be good to add a flag to the tree initialization that would allow incremental additions (similar to scipy.spatial.ConvexHull) and then require a finalization operation to be done before any other operations.

  • Interpolation is "lazy" 2nd order now for all E, CC, and F interpolation, meaning we triangulate the grid points to interpolate. Interpolating without triangulation on the TreeMesh is still a point of research. This is not as fast as it could be, but still faster than previously.

  • Both face and edge operations are defined for 2D, (basically a re-ording of x-edges -> y-faces)

  • Do not expect any ordering for the faces, edges, and nodes. They are whatever they have decided to be.

  • Do not expect any of the private members of the class to have remained consistent between implementations.

  • There are many other changes which should hopefully be apparent as you look through the code.

Big things that still need to be implemented

  • Serialization, (do not expect to pickle this object and have it work, however with construction being much faster, this shouldn't slow down anyone too much while it is being worked on).

  • PlotImage and PlotSlice.

There are also many areas that this code could be extended to handle different types of underlying meshs, as well as support for differing sizes of axes, but as I said before, the initial goal of this pull request is to mimic the behavior of the current implementation.

Other than that, I hope this speeds up the operations for those who need them.

Update

  • Serialization should be implemented now (the TreeMesh is pickle-able)
  • PlotImage and PlotSlice are also implemented.
  • It also now support differing dimensions of the underlying TensorMesh
  • Interpolation is now NOT triangulated, so do there is no longer 2nd order convergence of the non-node interpolation matrices.
  • the refine and insert_cells function now have a finalize keyword arg that can be set to false if you want to do multiple steps of tree building

Big things left to do: Serialization
Still need to fix up InnerProduct matrices Pxxx
Updated the Face and Edge interpolation matrices to accept vectors
@fourndo
Copy link
Member

fourndo commented May 2, 2018

Very excited about this. I ll try to hook my previous inversion scripts and let you know how it goes.
Thanks so much @jcapriot !!

the conda that is activated in appveyors VM is "clean" no need to set up a new test environment.
only testing on tree currently (to hopefully speed up tests)
@rowanc1 rowanc1 requested review from lheagy, rowanc1 and fourndo May 3, 2018 22:44
Creates both cellBoundaryInd and faceBoundaryInd arrays by looping over all of the cells and checking if it's neighbor in that direction is NULL
@lheagy
Copy link
Member

lheagy commented Jun 25, 2018

@jcapriot: this is looking solid! Let me know when you feel it is ready to go :)

This function will only work for CC, F, and E data, (which is what was supported before). It works by creating a temporary QuadTree mesh through the slice, and then uses the QuadTree mesh's plotImage function.
This should give a better error message instead of just silently continuing without a helpful message.
The OcTree mesh triangulation results in too many degenerate simplices, thus making the search slow.
For now just switch back to an implementation that mimics the previous version (should still be much faster)
All of the averaging matrices that are in the TensorMesh are now implemented for the TreeMesh
Also adding in the tests to ensure they are implemented correctly
@lheagy
Copy link
Member

lheagy commented Jul 11, 2018

I can take a look through the merge conflicts on this tonight

@lheagy lheagy mentioned this pull request Jul 12, 2018
@codecov
Copy link

codecov bot commented Jul 13, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@0e64f1a). Click here to learn what that means.
The diff coverage is 83.63%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #94   +/-   ##
=========================================
  Coverage          ?   76.29%           
=========================================
  Files             ?       17           
  Lines             ?     4151           
  Branches          ?        0           
=========================================
  Hits              ?     3167           
  Misses            ?      984           
  Partials          ?        0
Impacted Files Coverage Δ
discretize/TreeMesh.py 51.73% <ø> (ø)
discretize/utils/interputils.py 81.08% <0%> (ø)
discretize/Tests.py 78.64% <75%> (ø)
discretize/MeshIO.py 85.67% <95.55%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e64f1a...b45c8c7. Read the comment docs.

@fourndo
Copy link
Member

fourndo commented Jul 13, 2018

@jcapriot I am getting issues with the _aveCC2F Stencil.
First it is now a function instead of a property?
Dimensions have changed also. Can you double check?

jcapriot and others added 3 commits July 19, 2018 13:45
@lheagy lheagy merged commit 6be5bdf into master Jul 20, 2018
@lheagy lheagy deleted the TreeMesh_accel branch July 20, 2018 05:07
@lheagy lheagy mentioned this pull request Aug 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants