Test and merge CGAL setup tool

[schmie]

Reported by miguel on 17 Jul 2013 09:37 UTC None

[schmie]

Comment by miguel on 17 Jul 2013 10:52 UTC Installed CGAL 4.2 via homebrew: brew install cgal.

[schmie]

Modified by miguel on 17 Jul 2013 12:25 UTC

[schmie]

Comment by miguel on 17 Jul 2013 12:25 UTC I'm having trouble compiling.

CGAL 4.2 installed via homebrew doesn't seem to provide CGAL/Point_inside_polyhedron_3.h Compilation terminates with:

HemeLbSetupTool/Model/Generation/CGALtypedef.h:12:44: error: CGAL/Point_inside_polyhedron_3.h: No such file or directory

I noticed that there's a copy of that file in HemeLbSetupTool/Model/Generation. I applied the following diff

diff -r dde77197b18e Tools/setuptool/HemeLbSetupTool/Model/Generation/CGALtypedef.h
--- a/Tools/setuptool/HemeLbSetupTool/Model/Generation/CGALtypedef.h    Thu Jul 04 10:50:26 2013 +0100
+++ b/Tools/setuptool/HemeLbSetupTool/Model/Generation/CGALtypedef.h    Wed Jul 17 13:19:52 2013 +0100
@@ -9,7 +9,7 @@
 #include <CGAL/AABB_polyhedron_triangle_primitive.h>
 #include <CGAL/IO/Polyhedron_iostream.h>
 #include <CGAL/IO/Verbose_ostream.h>
-#include <CGAL/Point_inside_polyhedron_3.h>
+#include "Point_inside_polyhedron_3.h"
 #include <CGAL/squared_distance_3.h>
 #include <CGAL/Polyhedron_incremental_builder_3.h>

and got around the issue.

The next error I see is

HemeLbSetupTool/Model/Generation/Point_inside_polyhedron_3.h:52: error: r3t3_do_intersect_endpoint_position_visitor is not a member of CGAL::internal

CGAL::internal::r3t3_do_intersect_endpoint_position_visitor is defined in the copy of Triangle_3_Ray_3_do_intersect.h in HemeLbSetupTool/Model/Generation. Unfortunately, my gcc is picking the homebrew version of Triangle_3_Ray_3_do_intersect.h which doesn't define it.

Jens, any advice on how to get this to compile?

[schmie]

Comment by miguel on 19 Jul 2013 14:42 UTC Jens replied to this by e-mail. Problem sorted.

I took some time to update the setup tool OS X installation instructions in SetupTasks. Note that we no longer require the faff of having 32 bit versions of VTK to match 32 bit wxpython. One can get working 64 bit version of wxpython via homebrew as documented.

Tool compiles and runs. Currently testing on previously failing dataset.

Merge will be postponed until PV is submitted so we don't interfere with Rupert's work.

@Jens: do you have performance figures pre and post swap to CGAL.

[schmie]

Comment by miguel on 19 Jul 2013 15:07 UTC I'm getting the following error with the_special_two_Miguel_tubed_smoothed.pro

CGAL::Polyhedron_incremental_builder_3<HDS>::
lookup_halfedge(): input error: facet 419609 shares a halfedge from vertex 595909 to vertex 595941 with facet 380339.
Segmentation fault: 11

[schmie]

Comment by jens on 23 Jul 2013 08:14 UTC I think I understand what that is about. Can you send me that profile or attach it here.

[schmie]

Comment by miguel on 23 Jul 2013 10:05 UTC I just sent it to you over UCL dropbox. Thanks for looking into this.

[schmie]

Comment by miguel on 21 Aug 2013 20:15 UTC Performance figures CGAL implementation

time hemelb-setup-nogui the_special_two_Miguel_tubed_smoothed.pro --voxel 1e-5
Setup time: 3096.222614 s

[schmie]

Comment by miguel on 21 Aug 2013 21:33 UTC With original VTK implementation

Setup time: 48.821631 s

[schmie]

Comment by miguel on 22 Aug 2013 08:02 UTC I notice that the current code doesn't write out the gmy file. Should the patch below be applied to turn it back on?

diff -r f612ce8feb3d Tools/setuptool/HemeLbSetupTool/Model/OutputGeneration.py
--- a/Tools/setuptool/HemeLbSetupTool/Model/OutputGeneration.py Thu Aug 22 08:59:27 2013 +0100
+++ b/Tools/setuptool/HemeLbSetupTool/Model/OutputGeneration.py Thu Aug 22 09:00:30 2013 +0100
@@ -83,7 +83,7 @@
         """
         t = Timer()
         t.Start()
-        self.generator.Execute(self.skipNonIntersectingBlocks)
+        self.generator.Writefile(self.skipNonIntersectingBlocks)
         XmlWriter(self.profile).Write()
         t.Stop()
         print "Setup time: %f s" % t.GetTime()

[schmie]

Comment by miguel on 22 Aug 2013 15:40 UTC Some tweaks to get the setup tool to compile under Ubuntu 12.04 in 79e1e528d0f5

[schmie]

Comment by jens on 22 Aug 2013 16:14 UTC I made some changes that should significantly speedup the voxelisation and merged them into the branch. Unfortunately the voxelisation now fails with the default voxelsize. I am currently investigating why.

[schmie]

Comment by jens on 22 Aug 2013 16:46 UTC With the new changes to the ID

On my mac

Setup time: 45.934868 s

and

Setup time: 37.656335 s

with the original tool

Part of the difference comes from the building of the new polygon (maybe up to 5 sec).

Unfortunately the new version still fails with the default voxelsize :(

[schmie]

Comment by miguel on 23 Aug 2013 11:52 UTC That's a great improvement Jens. Thank you very much.

I wouldn't worry about the 5 seconds, it's a modest time for the most complex geometry we have ever dealt with.

My ultimate goal is discretising the geometry with voxel size 0.433 um. Unfortunately, I'm still getting an inconsistent fluidness error:

RuntimeError: Inconsistent fluidness detected between site index x: 869; y: 974; z: 37, position x: 1078.77; y: 1274.18; z: 3.99527, which is solid and site index x: 870; y: 974; z: 36, position x: 1079.77; y: 1274.18; z: 2.99527, which is fluid but found 0 intersections with the surface.

Do you know what problem are we currently hitting? If there's some pathological feature in the mesh, I could try modifying the surface generation step (more/less smoothing, finer/coarser surface triangulation).

[schmie]

Comment by jens on 23 Aug 2013 12:03 UTC Yes. The issue is that the surface has holes in it after ignoring the invalid vertexes. I was testing for this but the data structure has to be updated before this returns the right answer.

About the 5 sec. That is unavoidable and I'm not gonna anything. The point is that this will be independent of voxelsize and only dependent on the original mesh size i.e. it is a fixed offset for this geometry.

[schmie]

Comment by miguel on 23 Aug 2013 12:46 UTC I agree. Can you please remind me what makes a vertex invalid?

[schmie]

Comment by miguel on 30 Aug 2013 09:45 UTC Thanks to the improvements of the past few days, I can now discretise at 0.433um.

I noticed that one of the regression tests is failing. I investigated the issue a bit and found out that it is due to cut distances less than 1e-6 off, so I'm gonna ignore that and update the test fixture. This is the code I used for checking:

diff -r e91918263cdd Tools/setuptool/test/TestHemeLBSetupTool/Model/test_OutputGeneration.py
--- a/Tools/setuptool/test/TestHemeLBSetupTool/Model/test_OutputGeneration.py   Thu Aug 29 15:41:38 2013 +0100
+++ b/Tools/setuptool/test/TestHemeLBSetupTool/Model/test_OutputGeneration.py   Fri Aug 30 10:42:24 2013 +0100
@@ -33,6 +33,24 @@
         generator = OutputGeneration.PolyDataGenerator(p)
         generator.Execute()

+        stored_file = ConfigLoader(os.path.join(dataDir, 'test.gmy'))
+        stored_file.Load()
+        new_file = ConfigLoader(outGmyFileName)
+        new_file.Load()
+
+        assert sum(stored_file.Domain.BlockFluidSiteCounts) == sum(new_file.Domain.BlockFluidSiteCounts)
+        from itertools import izip
+        for s_block, n_block in izip(stored_file.Domain.Blocks, new_file.Domain.Blocks):
+            if s_block.Sites is not None:
+                for s_site, n_site in izip(s_block.Sites, n_block.Sites):
+                    assert s_site.IsEdge == n_site.IsEdge
+                    if n_site.IsEdge:
+                        assert np.all(s_site.Position == n_site.Position)
+                        assert np.all(s_site.IntersectionType == n_site.IntersectionType)
+                        assert np.all(np.abs(s_site.IntersectionDistance - n_site.IntersectionDistance) < 1e-6)
+            else:
+                assert s_block.Sites == n_block.Sites
+        
         import filecmp
         assert filecmp.cmp(outGmyFileName, os.path.join(dataDir, 'test.gmy'))
         assert filecmp.cmp(outXmlFileName, os.path.join(dataDir, 'test.xml'))

I'm gonna update the regression test, merge the branch, and close the ticket today or Monday.

I'm very pleased with this. Thanks for your efforts, Jens!

[schmie]

Comment by miguel on 1 Oct 2013 14:34 UTC Branch merged. Closing ticket.