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

Error handling #4

Closed
hfossli opened this issue Nov 18, 2016 · 10 comments
Closed

Error handling #4

hfossli opened this issue Nov 18, 2016 · 10 comments

Comments

@hfossli
Copy link

hfossli commented Nov 18, 2016

I see quite a few asserts here and there in the code. Does that mean that if for example the solver is overconstrained the program will crash and exit? Or if I try to remove a constraint which hasn't been added it will crash and exit?

@hfossli
Copy link
Author

hfossli commented Nov 18, 2016

You see I'm considering writing a Swift wrapper and this seems to be my biggest concern so far.

@starwing
Copy link
Owner

the assert() means never raise error, if assert() occurs, it's the coding bug and not a programming runtime error. any assertion failure should not occurs in production scene.

I have added auto test and coverage test, it's not 100%, that's the way I will work. for now in coverage amoeba should never crash on any input data.

@hfossli
Copy link
Author

hfossli commented Nov 19, 2016

Great!

@hfossli hfossli closed this as completed Nov 19, 2016
@hfossli
Copy link
Author

hfossli commented Nov 26, 2016

Overconstraining a variable fails with

Assertion failed: (exit.id != 0), function am_remove, file ./amoeba.h, line 963.

Example code

static void test_twice(void) {
    am_Solver *s7 = am_newsolver(debug_allocf, NULL);
    am_Constraint *c15 = am_newconstraint(s7, 1000000000.0);
    am_setstrength(c15, 1000000000.0);
    am_setrelation(c15, 2);
    am_Variable *v14 = am_newvariable(s7);
    am_addterm(c15, v14, 1.0);
    am_addconstant(c15, -10.0);
    am_add(c15);
    am_Constraint *c16 = am_newconstraint(s7, 1000000000.0);
    am_setstrength(c16, 1000000000.0);
    am_setrelation(c16, 2);
    am_addterm(c16, v14, 1.0);
    am_addconstant(c16, -10.0);
    am_add(c16);
}

It seems better if it instead

  • returns a proper error code e.g. AM_FAILED
  • leaves the solver as if am_add(c16) didn't happen at all

@hfossli hfossli reopened this Nov 26, 2016
@starwing
Copy link
Owner

I will test it.

@starwing
Copy link
Owner

to add a new constraint, we have several ways to try (via Cassowary algorithm):

  1. add it directly, if we find a subject.
  2. use a artificial variable to add.

if we cannot find a subject and all terms are dummy variable, and constant is not zero (means some constraint like x >= 10 when x <= 0), it's unsatisfied.

then, if we can not add it with a artificial variable, the constraint may unbound.

but, all other implements just throw errors in all these situations, I just try to restore the state of solver and return the proper error code. So I must remove the added constraint if needed, I need may test case on adding, because all other implement not do this.

If I'm not real add the constraint into solver and I try to remove it, this assertion failure will trigger, this is the source of this failure. I will improve it, but I need more test case to fix every the one case.

@hfossli
Copy link
Author

hfossli commented Nov 28, 2016

I just try to restore the state of solver and return the proper error code

Music to my ears!

I will improve it

Fantastic!

but I need more test case to fix every the one case

I can create more test cases. The test_twice test was created using my swift wrapper. I can enable logging of all actions to the c-api of amoeba so that it is easy to reproduce any errors I get. That's why the solver is named s7 :)

Your library is very easy to use. Your style of writing c code is very clean. I'm very impressed with your skills!

@starwing
Copy link
Owner

Very very thank you for your support :-)

@starwing
Copy link
Owner

I have fixed this assertion failure, you could try the latest version.

@hfossli
Copy link
Author

hfossli commented Nov 28, 2016

Perfect!

@hfossli hfossli closed this as completed Nov 28, 2016
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

No branches or pull requests

2 participants