-
-
Notifications
You must be signed in to change notification settings - Fork 117
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
Add extra exceptions for arithmetic and types operations #1352
Conversation
First it looks like there is another flaoting-point error see:
Second the thing does not work on OS X. I will have to turn it on only for Linux. |
Can you see the issue with |
@dcoeurjo OK, I will try to find a solution once I will have a bit of time. |
Thx.. Not easy to locate the actual bug with the SIGFPE signal which does not provide a lot of information |
@dcoeurjo Yup, I use GDB to stop at the signal and try to understand where is the problem. I think there are better techniques to deal with this. I will need to have a look at. |
I see.. thx anyway.. |
can you please merge with the master ? Are all tests passing now ? |
@dcoeurjo They should, lets see what travis will tell us right now. |
@dcoeurjo One test is still failing. The fix is in a non-merged branch: Curves3D. I am working on it. |
If #1339 is merged, will the remaining test be fine? |
ping @copyme |
@copyme it should be fine I will merge master and we will see |
@dcoeurjo sorry for the delay, I have not seen your message. I just added a changelog entry, and if the tests go then you can merge, I think. |
@dcoeurjo, in fact, there are bugs in the tests. The fix discovered new issues. I will have a look at and try to fix them. |
👌 |
@dcoeurjo I will need a hand to fix these issues. So, in
Since I do not understand the code well (and I have no time to) I need a piece of precise information on how to fix this issue. |
ping @cgurps |
You can replace this line by
The value doesn't really matter for this test |
Also, on my machine I get this:
But maybe this is Eigen version related since it works well on Travis. |
It is related to recent Boost versions (see #1268). |
@rolanddenis thanks for the info! |
@dcoeurjo I am done here. Just pay attention to the changes mentioned above while merging. |
all good.. thanks. |
This PR allows for extra exceptions in the debug mode (G++).
So, without these few lines we have all the test passing:
But if we run the tests with the additional exceptions we get:
The bugs related to LambdaMST3D are already fixed in #1339
It is not that bad only two tests failed (and one of mine). We can think about adding different types of extra exceptions but allowing all may cause problems with boost, for example.