raysect / source

The main source repository for the Raysect project.
http://www.raysect.org
BSD 3-Clause "New" or "Revised" License
86 stars 23 forks source link

Tetrahedral mesh for arbitrary 3D mesh #370

Closed munechika-koyo closed 3 years ago

munechika-koyo commented 3 years ago

Hello. I would like to provide the Discrete3DMesh class to make it possible to address the tetrahedral mesh because I want to take EMC3-EIRENE mesh as an arbitrary 3D mesh emitter. This class enables us to take account of tetrahedral mesh like the Discrete2DMesh if we give vertices, tetrahedra indices, and voxel data contained by one tetrahedral mesh. I also modified the kdtree3D class to facilitate doing so. I would appreciate it if you could comment and check if this feature is compatible with the current raysect. Thank you in advance.

The following example is the Stanford bunny tetrahedral mesh as a unit emitter. tetrahedra_mesh_emitter

vsnever commented 3 years ago

Hello @munechika-koyo, I started working with EMC3-Eirene data quite recently and can confirm that this contribution is really needed, so thank you very much. To create a plasma object we may need a Discrete3DMeshVector3D class as well, however this is a separate issue.

munechika-koyo commented 3 years ago

Thank you @vsnever for your comment. Yes, you would make use of it for cherab's plasma object as well.

CnlPepper commented 3 years ago

Wow, this is a very impressive first time contribution! 3D volume meshes have been one of the items we really desired, but did not have time to work on. I've had a quick scan through the code and very happy with the code style and layout thus far.

We will formally review it as soon as we can, though it may take us a little time to go through it. Our workload is high at present and the contribution is quite complex (potential corner cases etc). So please bear with us on that one.

Thanks for your contribution!

CnlPepper commented 3 years ago

It looks good, some minor comments to address. Once those are in place I'm happy to merge this. You did a great job of following the style of the existing codebase, made the review very easy.

munechika-koyo commented 3 years ago

Sorry for my late reply. Thank you @CnlPepper and @vsnever for reviewing my codes. I modified some points according to your comments, and I would appreciate it if you could confirm it.

munechika-koyo commented 3 years ago

I found that there was a bug in the tetrahedra mesh. When I set the simple tetrahedron: vertices = np.array([[1, 0, 1], [1, 0, 0], [1, 1, 0], [0, 0, 0]]) tetrahedra = np.array([[0, 1, 2, 3]], tetra = Discrete3DMesh(vertices, tetrahedra, np.ones(tetrahedras.shape[0]), False, 0)

and checked if some points on the tetrahedral surface was included, I obtained the following results:

points coordinates : True if the point is included (e.g. if tetra(1, 0, 0) == 1, True) (x, y, z) = (1.0, 0.0, 0.0): True (x, y, z) = (1.0, 0.3333333333333333, 0.0): False (x, y, z) = (1.0, 0.6666666666666666, 0.0): True (x, y, z) = (1.0, 1.0, 0.0): True. and (x, y, z) = (1, 0.333333333, 0): True (x, y, z) = (1, 0.3333333333, 0): False

These results are confusing me, and I am just suspecting the calculation accuracy or the cython

CnlPepper commented 3 years ago

At first glance this looks like numerical accuracy to me. If you zoom into the surface of a non axis-aligned triangle and perform an inside test at multiple points along the expected edge, you will find the edge of the triangle is ragged. This is due to the quantisation of the floating point numbers in the calculation due to the limited bit depth of the float/double.

munechika-koyo commented 3 years ago

I agree to @CnlPepper 's idea. When I modified the order of calculation of barycentric codes, some parts got good, and some parts didn't change. We have got no choice but to compromise with this issue...

vsnever commented 3 years ago

@munechika-koyo, could you please mark this PR as "ready for review" if it's ready? I think that merging is blocked for draft PRs.

munechika-koyo commented 3 years ago

@vsnever Sorry I didn't realize such a thing. I made it!

vsnever commented 3 years ago

I made it!

Thanks, now let's wait for @CnlPepper's decision.