deleteFirstSonBart deletes Bart from the wrong Family register

[georghinkel]

It took me a while to find out, but the deleteForstSonBart method used in the IncrementalForward actually deletes a Bart that is not necessarily part of the given register. It simply deletes a Bart that is part of the register instance some other helper method has created, but in my case, this is not the same because in the meantime, changes in the FamilyModel were propagated and I have to use a deserialized family register from a previous propagation.

[georghinkel]

The method actually looks like the correct done code somehow did not work and was replaced by a quick and (very) dirty fix:

                /*
        Family family = getSimpsonFamily(register);
        assertTrue(family.getName().equals("Simpson"));

        EcoreUtil.delete(family.getSons().get(0), true);
        */
                EcoreUtil.delete(firstBart, true);

[tbuchmann]

I wouldn't call it a very dirty fix. The problem is the following: The old code simply deleted the first son of a family regardless his name. Furthermore, we have test cases, where we have two sons with the name Bart. The FamilyHelper class is tailored towards our testcases, and it does not make sense to use them in any other place to build an instance of a family model.

If your tool works incrementally, the family register instance should not change during the test case execution, and thus, the FamilyHelper instance should reference the correct objects.

[georghinkel]

It does work incrementally, but the Family register that does not change is the one in the .NET process - not the one in the Java process. I could try to record the changes that I did to the target model and apply it to the Java version of the Family register, but that has multiple disadvantages:

1. Change recording adds some overhead. The NMF change recorder is currently implemented in such a way that it asks the model to report on changed URIs, which means that these changed URIs must be computed and parsed, which adds quite some overhead.
2. That would mean to implement another class to apply NMF Changes in Java. While such a class would be beneficial for the Smart Grid case as well, I feel it is bloating up the NMF solution to the Families to Persons case.

After all, if you claim that your benchmark also supports non-JVM solutions, I think you should rather accept that family register instance can change, because the one in the Java process may not be the primary one. Please remind that the TTC is not a contest only for JVM-based tools, but it is an academic contest and therefore should be open to all technologies.

[tbuchmann]

I understand your point. However, just uncommenting the commented code doesn't do the trick, because we might have multiple Simpson Families. I'll review all test cases and check if we can assume that the corresponding family is ALWAYS e.g. the second Simpson family and if so - change the code accordingly. Please give me time until friday, as i'm busy with teaching tomorrow.

[anthonyanjorin]

@tbuchmann You could try the following: (i) if the "old" bart object using JVM-based object identity can be located then we delete it (regardless of it's position in the list of sons). (ii) If this fails, i.e., no such Bart with the correct object identity can be found, then we revert to a position-based heuristic and hope that non-JVM tools do not scramble the lists (which they are currently free to do).

I think (i) would correspond to the current code, while (ii) would be the old code.

This test is a bit tricky as we do not want to introduce some kind of id attribute, but have to somehow identify the correct old Bart.

[anthonyanjorin]

@georghinkel Does the current fix (merged on master) work for you? If yes, you can close this issue.

Cheers and thanks, Tony

[georghinkel]

Yes, test is green now :)