Gecode / gecode

Generic Constraint Development Environment
https://www.gecode.org
Other
283 stars 76 forks source link

PropagatorGroup::size(..) returns a wrong value - OR?? #57

Closed k0stjap closed 5 years ago

k0stjap commented 5 years ago

When I run the following program:

/* g++ -o bug1 =1/bug1.cpp -L/usr/local/lib -lgecodekernel -lgecodesupport -lgecodeint -lgecodesearch setenv LD_LIBRARY_PATH /usr/local/lib ./bug1

*/

include <gecode/int.hh>

include <gecode/search.hh>

using namespace Gecode;

class Sp0 : public Space { public: IntVar iv0; IntVar iv1;

public: Sp0() : iv0(this, 0, 1), iv1(this, 0, 1) {} Sp0(Sp0 &s) : Space(s), iv0(this, 0, 1), iv1(this, 0, 1) {}

virtual Space copy(void) { return new Sp0(this); }

IntVar getIV0() { return (iv0); } IntVar getIV1() { return (iv1); } };

// int main(int argc, char* argv[]) { PropagatorGroup pg;

Sp0 sp = new Sp0(); Home spg = (sp)(pg); rel(spg, sp->getIV0(), IRT_EQ, sp->getIV1()); (void) sp->status(); std::cout << "pg.size(sp) = " << pg.size(sp) << std::endl;

Sp0 sp1 = dynamic_cast<Sp0 >(sp->clone()); (void) sp1->status(); std::cout << "pg.size(sp1) = " << pg.size(sp1) << std::endl;

rel(sp1, sp->getIV0(), IRT_EQ, 1); (void) sp1->status(); std::cout << "sp1->iv1 = " << sp1->getIV1() << std::endl; std::cout << "pg.size(sp1) = " << pg.size(*sp1) << std::endl;

rel(sp, sp->getIV0(), IRT_EQ, 1); (void) sp->status(); std::cout << "sp->iv1 = " << sp->getIV1() << std::endl; std::cout << "pg.size(sp) = " << pg.size(*sp) << std::endl;

return (0); }

the output is (gecode 6.1.1, (up-to-date) gentoo on an x86_64):

pg.size(sp) = 1 pg.size(sp1) = 1 sp1->iv1 = [0..1] pg.size(sp1) = 1 sp->iv1 = 1 pg.size(sp) = 0

Note that the space cloning is no cloning here - purposefully for the sake of a lean example. Instead, new variables are created, and so should the propagator in 'sp' be missing in 'sp1' - which it does, according to the observed behaviour, yet it somehow remains counted.

Did I miss anything?

Cheers, --- Kostja.

chschulte commented 5 years ago

You are not allowed to create new variables or propagators (or anything) during cloning. You are just allowed to update the variables.

k0stjap commented 5 years ago

Thanks for the response!

First of all, - surely your ".. just allowed to update the variables" statement does NOT imply that all gecode variables (or variable arrays) that are part of the CSP solution must be immediate members of a used-defined Space sub-class - and in particular, they cannot be heap-allocated separately (stand-alone or as part(s) of some other structures - with necessary pointer(s) from the Space object), does it??

I am sorry - but where in the Gecode document did I miss the discussion of what the Space copy constructor for cloning is allowed resp. not allowed to do?

My specific "use case" involves an incremental construction of a CSP - i.e. incremental extension of the model by new variables and constraints, and solving a sequence of fixed-size "partial" sub-problems - i.e. those involving a subset of variables and only relevant constraints. Very roughly speaking, the loop consists of defining some new variables and constraints, then selecting a sub-problem to solve and solving it, and then constraining the original full" CSP using parts of the sub-problem results. In this setting, solving a sub-problem starts by a form of "selective" copying of the full CSP - which ignores all irrelevant variables, and, therefore, all constraints on those should just "get lost" in the process of copying.

The following program just drops references to heap-allocated variables during copying - yet the propagator group still accounts for the propagator:

// g++ -o bug2 =1/bug2.cpp -L/usr/local/lib -lgecodekernel -lgecodesupport -lgecodeint -lgecodesearch // setenv LD_LIBRARY_PATH /usr/local/lib // ./bug2

include <gecode/int.hh>

include <gecode/search.hh>

using namespace Gecode;

class Sp0 : public Space { public: IntVar iv0p; IntVar iv1p;

public: Sp0() : iv0p(new IntVar(this, 0, 1)), iv1p(new IntVar(this, 0, 1)) {} Sp0(Sp0 &s) : Space(s), iv0p(NULL), iv1p(NULL) {}

virtual Space copy(void) { return new Sp0(this); }

IntVar getIV0p() { return (iv0p); } IntVar getIV1p() { return (iv1p); } };

// int main(int argc, char* argv[]) { PropagatorGroup pg;

Sp0 sp = new Sp0(); Home spg = (sp)(pg); rel(spg, (sp->getIV0p()), IRT_EQ, (sp->getIV1p())); (void) sp->status(); std::cout << "pg.size(sp) = " << pg.size(sp) << std::endl;

Sp0 sp1 = dynamic_cast<Sp0 >(sp->clone()); (void) sp1->status(); std::cout << "pg.size(sp1) = " << pg.size(sp1) << std::endl; // Segmentation fault : there are no variables in 'sp1'; // rel(sp1, (sp1->getIV0p()), IRT_EQ, 1);

rel(sp, (sp->getIV0p()), IRT_EQ, 1); (void) sp->status(); std::cout << "sp->iv1 = " << (sp->getIV1p()) << std::endl; std::cout << "pg.size(sp) = " << pg.size(*sp) << std::endl;

return (0); }

which produces:

pg.size(sp) = 1 pg.size(sp1) = 1 sp->iv1 = 1 pg.size(*sp) = 0

where, specifically, 'pg.size(*sp1)' should be 0, not 1.

chschulte commented 5 years ago

The variables that are used by constraints and branchers are copied by them, so you only need to declare those variables in a Space you are actually interested in.

Yep, occurred to me that the doc is a little hazy here, will improve…

Don’t heap allocate variables! Really, that’s not going to work and there is no need for it. Each Space does its own memory management.

Cheers Christian

-- Christian Schulte, https://chschulte.github.io/ Professor of Computer Science Software and Computer Systems School of Electrical Engineering and Computer Science KTH Royal Institute of Technology, Sweden

From: k0stjap notifications@github.com Sent: Tuesday, October 8, 2019 4:03 PM To: Gecode/gecode gecode@noreply.github.com Cc: Christian Schulte cschulte@kth.se; State change state_change@noreply.github.com Subject: Re: [Gecode/gecode] PropagatorGroup::size(..) returns a wrong value - OR?? (#57)

Thanks for the response!

First of all, - surely your ".. just allowed to update the variables" statement does NOT imply that all gecode variables (or variable arrays) that are part of the CSP solution must be immediate members of a used-defined Space sub-class - and in particular, they cannot be heap-allocated separately (stand-alone or as part(s) of some other structures - with necessary pointer(s) from the Space object), does it??

I am sorry - but where in the Gecode document did I miss the discussion of what the Space copy constructor for cloning is allowed resp. not allowed to do?

My specific "use case" involves an incremental construction of a CSP - i.e. incremental extension of the model by new variables and constraints, and solving a sequence of fixed-size "partial" sub-problems - i.e. those involving a subset of variables and only relevant constraints. Very roughly speaking, the loop consists of defining some new variables and constraints, then selecting a sub-problem to solve and solving it, and then constraining the original full" CSP using parts of the sub-problem results. In this setting, solving a sub-problem starts by a form of "selective" copying of the full CSP - which ignores all irrelevant variables, and, therefore, all constraints on those should just "get lost" in the process of copying.

The following program just drops references to heap-allocated variables during copying - yet the propagator group still accounts for the propagator:

// g++ -o bug2 =1/bug2.cpp -L/usr/local/lib -lgecodekernel -lgecodesupport -lgecodeint -lgecodesearch // setenv LD_LIBRARY_PATH /usr/local/lib // ./bug2

include <gecode/int.hh>

include <gecode/search.hh>

using namespace Gecode;

class Sp0 : public Space { public: IntVar iv0p; IntVar iv1p;

public: Sp0() : iv0p(new IntVar(this, 0, 1)), iv1p(new IntVar(this, 0, 1)) {} Sp0(Sp0 &s) : Space(s), iv0p(NULL), iv1p(NULL) {}

virtual Space copy(void) { return new Sp0(this); }

IntVar getIV0p() { return (iv0p); } IntVar getIV1p() { return (iv1p); } };

// int main(int argc, char* argv[]) { PropagatorGroup pg;

Sp0 sp = new Sp0(); Home spg = (sp)(pg); rel(spg, (sp->getIV0p()), IRT_EQ, (sp->getIV1p())); (void) sp->status(); std::cout << "pg.size(sp) = " << pg.size(sp) << std::endl;

Sp0 sp1 = dynamic_cast<Sp0 >(sp->clone()); (void) sp1->status(); std::cout << "pg.size(sp1) = " << pg.size(sp1) << std::endl; // Segmentation fault : there are no variables in 'sp1'; // rel(sp1, (sp1->getIV0p()), IRT_EQ, 1);

rel(sp, (sp->getIV0p()), IRT_EQ, 1); (void) sp->status(); std::cout << "sp->iv1 = " << (sp->getIV1p()) << std::endl; std::cout << "pg.size(sp) = " << pg.size(*sp) << std::endl;

return (0); }

which produces:

pg.size(sp) = 1 pg.size(sp1) = 1 sp->iv1 = 1 pg.size(*sp) = 0

where, specifically, 'pg.size(*sp1)' should be 0, not 1.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHubhttps://github.com/Gecode/gecode/issues/57?email_source=notifications&email_token=ADL6DBJPJZIVRD6VK3YCAP3QNSHI7A5CNFSM4I32YM42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAUI5AY#issuecomment-539528835, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ADL6DBJHFYEK7CE4FSQBRBTQNSHI7ANCNFSM4I32YM4Q.

k0stjap commented 5 years ago

Dear Christian,

the "Don’t heap allocate variables! .." paragraph is too concise for me, unfortunately, to be useful - but the issue is very important already now (compared to the behaviour of 'PropagatorGroup'): as I mentioned, my CSP is constructed incrementally (and with certain randomness) - therefore, in particular, Gecode variables simply do not exist at the start, and are being added in the process.

So what exactly "is not going to work", and under which conditions, when I heap-allocate variables?? For example, can those dynamially created variables be saved in memory provided by some gecode "Space-centric" memory manager - instead of common heap?

Another issue I wish were covered in the programming guide (and/or the online documentation) is what is resp. is not allowed to happen in functions passed to Gecode::wait(..), specifically - adding branchers (I managed to be blessed with 'SpaceNoBrancher' exceptions while using the DFS, even though I had some test programs before writing the application code itself ;-))

I just hope I did not miss any "hidden" message from your first paragraph (which, in itself, I think I did understand after reading the programming guide).

chschulte commented 5 years ago

Hi,

The documentation makes clear that variables belong to a space, so there is really no point to store them outside a space. And new is not needed anyway as they are just pointers. I will try to disable new/delete in the next release to make that clear.

For a variable to be valid the space must still exist and hence it is easiest to store them as member of a space (you can think of a space as the closure for the variables).

No restriction on Gecode::wait.

Cheers Christian

-- Christian Schulte, https://chschulte.github.io/ Professor of Computer Science Software and Computer Systems School of Electrical Engineering and Computer Science KTH Royal Institute of Technology, Sweden

From: k0stjap notifications@github.com Sent: Wednesday, October 9, 2019 9:16 AM To: Gecode/gecode gecode@noreply.github.com Cc: Christian Schulte cschulte@kth.se; State change state_change@noreply.github.com Subject: Re: [Gecode/gecode] PropagatorGroup::size(..) returns a wrong value - OR?? (#57)

Dear Christian,

the "Don’t heap allocate variables! .." paragraph is too concise for me, unfortunately, to be useful - but the issue is very important already now (compared to the behaviour of 'PropagatorGroup'): as I mentioned, my CSP is constructed incrementally (and with certain randomness) - therefore, in particular, Gecode variables simply do not exist at the start, and are being added in the process.

So what exactly "is not going to work", and under which conditions, when I heap-allocate variables?? For example, can those dynamially created variables be saved in memory provided by some gecode "Space-centric" memory manager - instead of common heap?

Another issue I wish were covered in the programming guide (and/or the online documentation) is what is resp. is not allowed to happen in functions passed to Gecode::wait(..), specifically - adding branchers (I managed to be blessed with 'SpaceNoBrancher' exceptions while using the DFS, even though I had some test programs before writing the application code itself ;-))

I just hope I did not miss any "hidden" message from your first paragraph (which, in itself, I think I did understand after reading the programming guide).

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHubhttps://github.com/Gecode/gecode/issues/57?email_source=notifications&email_token=ADL6DBODV7MXFCVMQJLEI3LQNWAMVA5CNFSM4I32YM42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAW4V7Y#issuecomment-539871999, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ADL6DBJETRBCOVEENMR6DP3QNWAMVANCNFSM4I32YM4Q.