martindevans / Cassowary.net

An incremental constraint solver for .NET
http://jozilla.net/Software/CassowaryNet
GNU Lesser General Public License v2.1
3 stars 5 forks source link

Edit context variable not found #1

Open martindevans opened 9 years ago

martindevans commented 9 years ago

The following test (sent to me in an email) fails:

//Create a solver
var solver = new ClSimplexSolver();
var variable = new ClVariable(0f);

//Add a stay, indicating this var should stay at it's current value if possible
solver.AddStay(variable, ClStrength.Strong, 1);

const double EXPECTED_VALUE = 10.0;

//Suggest a value for the variable in an edit context
var editContext = solver.BeginEdit(variable);
editContext.SuggestValue(variable, EXPECTED_VALUE);
editContext.EndEdit();

//Assert that the value has changed to the suggested value
Assert.AreEqual(EXPECTED_VALUE, variable.Value);
martindevans commented 9 years ago

Can not reproduce this issue in the latest version of the library this is forked from (jozilla/Cassowary.net). This means it's my issue.

martindevans commented 9 years ago

Reproduced after the very first commit to this fork (9e195c723ed12c8e09872cf93d6039f81fb88257)

martindevans commented 9 years ago

Traced through code executed by unit test, cannot find any significant differences between old and new code which could introduce the bug (I really wish I had used smaller commits back then). Try re-applying changes file by file to narrow it down.

helmesjo commented 8 years ago

Any progress on this? Currently putting my progress to a halt.. :) Thanks! @martindevans

martindevans commented 8 years ago

I haven't made any progress on this unfortunately, it's definitely on my todo list though!

As a workaround how about trying this:

//Replace this...
solver.AddStay(variable, ClStrength.Strong, 1);

//With this...
solver.AddConstraint(variable, v => v == 1, ClStrength.Strong)

For stays I think this is exactly equivalent. I have actually considered removing stays entirely, since you can do this.

//Replace this...
solver.BeginEdit(variable).SuggestValue(variable, 1).EndEdit();

//With this...
solver.AddConstraint(variable, v => v == 1, ClStrength.Weak);

For an edit this isn't quite the same, but personally I prefer it to using suggest value.

Feel free to poke me again if this doesn't work for you :)

helmesjo commented 8 years ago

Alright thanks, I'll check it out and get back to you! :)

helmesjo commented 8 years ago

The changes you suggested did remove the exception, however the "new suggested value" won't be solved for. So this:

var solver = new ClSimplexSolver();
var variable = new ClVariable(0f);
solver.AddConstraint(variable, v => v == variable.Value, ClStrength.Strong);
solver.AddConstraint(variable, v => v == 10, ClStrength.Weak);

will not solve for 10, instead variable.value will still be 0.

This however, oddly enough (can't check the implementation atm so not exactly sure what's different in the overloaded method), works:

var solver = new ClSimplexSolver();
var variable = new ClVariable(0f);
solver.AddConstraint(variable, v => v == variable.Value, ClStrength.Strong);
solver.AddConstraint(variable, v => v == 10);
helmesjo commented 8 years ago

Okey, just checked it up quickly anyways.. And it's because default is ClStrength.Required. So the latter is equivalent to this, which also works:

var solver = new ClSimplexSolver();
var variable = new ClVariable(0f);
solver.AddConstraint(variable, v => v == variable.Value, ClStrength.Strong);
solver.AddConstraint(variable, v => v == 10, ClStrength.Required);

How will this differ to the SuggestValue-approach? :)

martindevans commented 8 years ago

This makes perfect sense, it's to do with strengths. In the first example you set 2 constraints:

Naturally we can't solve both those constraints, so you have to solve the stronger one. Value starts at zero, and you've set a strong constraint to set it to zero, so it stays at zero.

In the second example, you miss out the strength for the second constraint. The default strength is Required so this is equivalent to:

Again, both constraints can't be satisfied so we eliminate the weakest request. This time the second constraint beats out the first and so v is set to 10.

This perfectly demonstrates why I don't really like Stays and Edits - you trivially end up in a situation where not all constraints can be satisfied and strengths come into play, which is easy to forget.

martindevans commented 8 years ago

Further to that, I don't think v == variable.Value actually has any effect!

Honestly I'm not certain how this approach differs to SuggestValue (remember I didn't write the library, I just modernised it from very old style C#).

martindevans commented 8 years ago

What exactly are you actually trying to achieve? You can probably do it with just constraints (the best tested part of the library).

helmesjo commented 8 years ago

Alright, then I misinterpreted your first answer!

v == variable.Value was just to constrain it to the value of the variable, so no magic intended there!

I'm just trying to suggest a new value for a variable, which may or may not be constrained and if so try to solve for it! :) My understanding was that the whole edit->suggest->endedit really just changed the variable to weak (aka editable) and then trying to solve for the value best it can, correct? That is what I'm trying to achieve! :)

Note: I have no knowledge what so ever of Cassowary (more than what you put in and get out), I'm just implementing it to my needs! :)

Anyways, thanks for the help! I think I'll get it to do what I need as for now!

martindevans commented 8 years ago

I think the best way to achieve that is to set weak constraints to the values you'd like to suggest.

helmesjo commented 8 years ago

Wait yeah, this won't work as SuggestValue... :)

Yeah the problem is however that variables must be able to be constrained with any strength (except required I guess), then changed to a new value if necessary. Say the corners of a rect being stationary (strong), then if (while) the rect is dragged the new values will be suggested and solved for (thus the corners being weak), then when not dragged anymore they should be stationery (strong again). Which will work with the SuggestValue approach.

martindevans commented 8 years ago

I think see I what you mean. One further approach you could take is to use this constructor:

public ClVariable(string name, double value)

for variables, with whatever initial values you like. If you reconstruct your ClVariables with each solving "pass" you can effectively get the behaviour you want that way (if I've understood you correctly).