Skip to content

Add extra exceptions for arithmetic and types operations #1001

Merged
jdumas merged 3 commits intolibigl:masterfrom
copyme:ArithmExcept
Apr 11, 2020
Merged

Add extra exceptions for arithmetic and types operations #1001
jdumas merged 3 commits intolibigl:masterfrom
copyme:ArithmExcept

Conversation

@copyme
Copy link
Copy Markdown
Contributor

@copyme copyme commented Nov 8, 2018

This PR allows for extra exceptions in the debug mode (Linux only).

So, without these few lines we have all the test passing:


Running tests...
Test project /home/kacper/Projects/libigl/cmake-build-debug
    Start 1: run_igl_tetgen_tests
1/3 Test #1: run_igl_tetgen_tests .............   Passed    0.01 sec
    Start 2: run_igl_comiso_tests
2/3 Test #2: run_igl_comiso_tests .............   Passed   23.02 sec
    Start 3: run_igl_tests
3/3 Test #3: run_igl_tests ....................   Passed   26.59 sec

100% tests passed, 0 tests failed out of 3

Total Test time (real) =  49.61 sec

But if we run the tests with the additional exceptions we get:

Running tests...
Test project /home/kacper/Projects/libigl/cmake-build-debug
    Start 1: run_igl_tetgen_tests
1/3 Test #1: run_igl_tetgen_tests .............   Passed    0.01 sec
    Start 2: run_igl_comiso_tests
2/3 Test #2: run_igl_comiso_tests .............   Passed   22.68 sec
    Start 3: run_igl_tests
3/3 Test #3: run_igl_tests ....................***Exception: Numerical  0.46 sec

67% tests passed, 1 tests failed out of 3

Total Test time (real) =  23.16 sec

The following tests FAILED:
          3 - run_igl_tests (NUMERICAL)
Errors while running CTest
make: *** [Makefile:119: test] Error 8

Sometimes, it is not that easy to find bugs related to arithmetic and type errors so it will take me a while to find them and fix. But as you can see there is something wrong. So, stay tuned.

This PR allows for extra exceptions in the debug mode (GCC/LLVM) on L…
6b2da9a 
…inux based OSes - it does not work on the other operating systems.

Relevant documentation: https://linux.die.net/man/3/feenableexcept
@copyme
Copy link
Copy Markdown
Contributor Author

copyme commented Nov 8, 2018 •
edited
Loading

The first problem comes somewhere from tests/include/bbw.cpp. But, it is not so clear so far what is going on. You have there multi-thread operations which do not make things easier to debug. I will have a look at this during the weekend.

@copyme
Copy link
Copy Markdown
Contributor Author

copyme commented Nov 8, 2018 •
edited
Loading

And yes, before merging this PR all the bugs detected with it should be fixed. If you decide to merge, of course.

@jdumas jdumas changed the base branch from master to dev November 8, 2018 13:28
@jdumas
Copy link
Copy Markdown
Collaborator

jdumas commented Nov 8, 2018

Hi. It's a good idea. But before this can be merged it needs to be rebased onto the dev branch, where we've switched to Catch2 for unit testing. If you want to help you may also want to take a look at #996, which enables also CGAL for compilation on Travis and AppVeyor. Unfortunately in #996 it seems some tests are randomly failing on macOS due to memory issues (at least one buffer overflow in Eigen).

@copyme
Copy link
Copy Markdown
Contributor Author

copyme commented Nov 8, 2018 •
edited
Loading

@jdumas OK. I will have a look at these things.

This was referenced Nov 8, 2018
Closed
Closed
@copyme
Copy link
Copy Markdown
Contributor Author

copyme commented Nov 8, 2018

@jdumas just a quick question. Is libigl supposed to compile with CGAL on system without static boost (only dynamic)? OpenSUSE does not ship static boost and I can compile CGAL or libdgtal + cgal without problem but igl+CGAL even with set( Boost_USE_STATIC_LIBS OFF) does not compile - I have a cmake message saying that some boost components were not found.

@jdumas
Copy link
Copy Markdown
Collaborator

jdumas commented Nov 8, 2018

We do set the option CGAL_Boost_USE_STATIC_LIBS to ON by default (because we try to compile everything statically to avoid dll nightmare on Windows). Try to turn it off maybe.

@copyme
Copy link
Copy Markdown
Contributor Author

copyme commented Nov 8, 2018

@jdumas There is something strange in cmake files when CGAL_Boost_USE_STATIC_LIBS is set to OFF then boot libraries are not searched. I had to set them by hand. It works with other libraries and cgal itself.

@jdumas
Copy link
Copy Markdown
Collaborator

jdumas commented Nov 8, 2018

Are you trying that with a clean CMake folder? I mean we are just deferring to CGAL when to comes to searching for Boost, so it's probably a CGAL issue. Note that when you have the libraries installed system-wide (both Boost and CGAL), the CMake detection is usually much easier.

@copyme copyme mentioned this pull request Nov 8, 2018
Closed
@jdumas jdumas changed the base branch from dev to master April 11, 2020 17:03
@jdumas jdumas merged commit f029832 into libigl:master Apr 11, 2020
This was referenced Apr 12, 2020
Merged
Closed
skoch9 pushed a commit to skoch9/libigl that referenced this pull request Jun 3, 2020
@copyme @skoch9
Add extra exceptions for arithmetic and types operations (libigl#1001)
5654337 
* This PR allows for extra exceptions in the debug mode (GCC/LLVM) on Linux based OSes - it does not work on the other operating systems.
Relevant documentation: https://linux.die.net/man/3/feenableexcept
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdumas jdumas jdumas approved these changes

Assignees

No one assigned

Labels

unit-tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@copyme @jdumas