ATL-Research / incremental-class2relational

1 stars 7 forks source link

URI format of elements #25

Open georghinkel opened 1 year ago

georghinkel commented 1 year ago

The comparator finds that nothing from the NMF solution I am building is correct, but inspecting the XMI manually, there is conceptually nothing wrong. This is for example the result for the correctness1 test:

<?xml version="1.0" encoding="utf-8"?>
<xmi:XMI xmi:version="2.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xmi="http://www.omg.org/XMI">
  <relational_:Type name="Integer" xmlns:relational_="Relational" />
  <relational_:Type name="String" xmlns:relational_="Relational" />
  <relational_:Table key="#//2/@col.0" name="Family" xmlns:relational_="Relational">
    <col keyOf="#//2" type="#//0" name="objectId" />
    <col type="#//1" name="name" />
  </relational_:Table>
  <relational_:Table key="#//3/@col.0" name="Member" xmlns:relational_="Relational">
    <col keyOf="#//3" type="#//0" name="objectId" />
    <col type="#//1" name="firstName" />
    <col type="#//0" name="closestFriendId" />
  </relational_:Table>
  <relational_:Table name="Family_members" xmlns:relational_="Relational">
    <col type="#//0" name="FamilyId" />
    <col type="#//0" name="membersId" />
  </relational_:Table>
  <relational_:Table name="Member_emailAddresses" xmlns:relational_="Relational">
    <col type="#//0" name="MemberId" />
    <col type="#//1" name="emailAddresses" />
  </relational_:Table>
</xmi:XMI>

There are two important differences to the reference expected.xmi, though:

  1. The format of the URI is different. Does the comparator support the formatting #//0 or does it have to be /0? Would it be possible to support the format #//0 as well? Is that specified somewhere how the format has to be in these cases?
  2. The order of the model elements is different. Here, I would strongly suggest that this is then a false-positive of the comparator.
TheoLeCalvar commented 1 year ago

The format of the URI is different. Does the comparator support the formatting #//0 or does it have to be /0? Would it be possible to support the format #//0 as well? Is that specified somewhere how the format has to be in these cases?

The comparator is written in java and uses EMF. I don't think we did anything special to load the file. I'll look into it tomorrow to see if there is an option to pass to the loader to accept this format.

Ping @fjouault for order. Do you know if you could modify the model comparator to ignore order ?

fjouault commented 1 year ago

Some Context

When directly used to compare EObject collections, SimpleEMFModelComparator can ignore order given an appropriate localIdGetter (see https://github.com/ATL-Research/EMFModelFuzzer/blob/2b952b4cd2b53efaf854b02d775f9b7c5a82c947/lib/src/main/java/io/github/atlresearch/emfmodelfuzzer/SimpleEMFModelComparator.xtend#LL28C163-L28C176).

This localIdGetter must return an Object that acts as key in a HashMap, given an index, and an Object. It is only used to recognize equivalent elements in collections. The returned id does not need to be global to the model, only to each collection. By default, when comparing Resources, the index is used as id, which enforces order (see https://github.com/ATL-Research/EMFModelFuzzer/blob/2b952b4cd2b53efaf854b02d775f9b7c5a82c947/lib/src/main/java/io/github/atlresearch/emfmodelfuzzer/SimpleEMFModelComparator.xtend#LL13C88-L13C107).

Because Comparator calls Resource comparison (see https://github.com/ATL-Research/incremental-class2relational/blob/48170534086bc91a8f1b8f8c505ff26a18cd6404/utils/Comparator/src/main/java/atl/research/Comparator.java#L51), order is enforced. However, we could either improve the SimpleEMFModelComparator API to accept a localIdGetter when comparing Resources, or call collection comparison on Resource contents in Comparator.

Problem

The hard part is to compute appropriate ids. In the context of model transformation, in my experience, the best way to get ids for comparison is to leverage trace links. We have tested this to work quite well with SimpleEMFModelComparator. It was actually our original intent to use trace links, which are typically available in memory after running a transformation, in Comparator. However, in order to make the Comparator work with non-EMF tools, such as NMF, we decided to perform the comparison on serialized models, at a point where we do not have trace links any more.

In order to solve this issue, we would therefore have to require trace link serialization, and to define a format (probably just XMI with an appropriate metamodel) for that.

Would requiring trace link serialization be acceptable?

georghinkel commented 1 year ago

Hm, I honestly don't understand why on earth everybody is trying to calculate a diff when all they need is to check whether two models are equivalent. Calculating a diff when collections have no defined order is hard, but checking whether they are equivalent is not really. If it helps, I could create a small application to sign the expected models and then check that signature on the result.

A trace link model would also be fine for me, though I fear it makes the comparison a bit unfair towards proper model transformation tools as this somewhat artificial requirement would enforce the existence of a trace.

georghinkel commented 1 year ago

Plus, the names are always present and should be unique, isn't it?

georghinkel commented 1 year ago

I removed the #/ and now it works.

fjouault commented 1 year ago

SimpleEMFModelComparator only tries to find one difference, and does not compute a diff.

In the case of the Relational metamodel, names could indeed be used as local ids.

What do you think of pull request #28 (which pull request #30 tries to further improve)?