osate / osate2

Open Source AADL2 Tool Environment
http://osate.org
Eclipse Public License 2.0
36 stars 8 forks source link

Missing End to End Flow Check #879

Closed joeseibel closed 6 years ago

joeseibel commented 7 years ago

The end to end flow in the following model cannot be instantiated:

package pkg1
public
  abstract a1
  end a1;

  abstract implementation a1.i
    subcomponents
      sub1: abstract a3;
      sub2: abstract a2.i;
      sub3: abstract a3;
    connections
      conn1: feature sub1.af3 -> sub2.af1;
      conn2: feature sub2.af2 -> sub3.af3;
    flows
      etef1: end to end flow sub1 -> conn1 -> sub2.fpath1 -> conn2 -> sub3;
  end a1.i;

  abstract a2
    features
      af1: feature;
      af2: feature;
    flows
      fpath1: flow path af1 -> af2;
  end a2;

  abstract implementation a2.i
    subcomponents
      sub4: abstract a3;
    connections
      conn3: feature af1 -> sub4.af3;
      conn4: feature sub4.af3 -> af2;
--  flows
--    fpath1: flow path af1 -> conn3 -> sub4 -> conn4 -> af2;
  end a2.i;

  abstract a3
    features
      af3: feature;
  end a3;
end pkg1;

The connections and flow specification don't form a connected path. If the commented out flow implementation is put back in, then an End to End Flow Instance is created. Currently, this model passes validation and the instantiator doesn't complain, it simply doesn't create the flow. An error message should be produced, but should the error message be generated by the validator or the instantiator?

lwrage commented 6 years ago

The situation is as follows:

15119891693701665929086

AaronGreenhouse commented 6 years ago

The error here is that a2.i is required to provide a flow implementation, but is not.

Section 10.2, paragraph (1): "Component implementations must provide an implementation for each flow specification."

The validator should be catching this, but is not.

But, the flow spec is present for a2, so there is no reason the end to end flow cannot use this information.

AaronGreenhouse commented 6 years ago

Using the flow path implementation

  fpath1: flow path af1 -> af2;

in a2.i doesn't work either. A warning is generated that it "doesn't add value to the model" and the end to end flow is not generated, even though this is a perfectly legal flow path implementation.

Warning makes sense most of the time, but end to end flow should definitely be generated. Warning is a little problematic, because ultimately there has to be an inner most component that doesn't have any subcomponents and whose flow implementations are going to be trivial. In a fully specified system you wouldn't really want to have a warning for this case.

AaronGreenhouse commented 6 years ago

So we have at least two problems here

  1. No error message if an implementation doesn't implement a flow from the type
  2. No end to end flow is generated in the instance model if the flow path component is trivial
AaronGreenhouse commented 6 years ago

I have implemented a check in Aadl2JavaValidator for the first problem.

lwrage commented 6 years ago

The added validation is fine. Te second issue is that the connection instance ends at sub4 but the flow spec (and your added implementation fpath1: flow path af1 -> af2;) don't have anything to do with sub4. It's OK that the e2e flow is not instantiated. A validation can probably not catch this case.

reteprelief commented 6 years ago

The trivial flow implementation should only be used when there are no subcomponents. To be more precise if the incoming feature has a connection declaration to a subcomponent then the flow implementation must include the subcomponent. In other words, we may have a component where some features are connected to subcomponents and other features are not.

lwrage commented 6 years ago

@reteprelief What if the incoming feature is connected to two subcomponents? If we don't require flow implementations for each possible path we can still end up with an unconnected case that probably cannot be found by a validation.

AaronGreenhouse commented 6 years ago

Seems like we need to be able to generate an end to end flow when a flow is trivial, because there must be a trivial flow at some point. Even if we use the flow

fpath1: flow path af1 -> conn3 -> sub4 -> conn4 -> af2;

in the above example, there is an implicit trivial flow through sub4.

lwrage commented 6 years ago

Correct. The 'trivial flow' thing is a red herring. The issue is that everything must be connected in the expanded end to end flow. In the original example this is not the case, but no error is reported. If we add a trivial flow implementation things are still not properly connected. With fpath1: flow path af1 -> conn3 -> sub4 -> conn4 -> af2; everything is connected and the flow can be instantiated.

lwrage commented 6 years ago

The end to end flow etef1 in the original example specifies that sub2.fpath1 must be part of the end to end flow instance (because there is no flow implementation that further decomposes the path). However, the connection instances and at a feature of sub4, so they don't connect to the ends of sub2.fpath1. This is clearly a problem in the AADL model that should be reported as an error.

AaronGreenhouse commented 6 years ago

Okay, I'm a bit confused as to what the proposed solution here is.

As I said, I have updated the validator to flag an error if a compoment implementation doesn't provide a flow implementation for each flow specification in the implemented type. (Although this is ignoring the case of modes.)

Regarding the sanity of the flow, I am not sure what if anything, is supposed to be done.

lwrage commented 6 years ago

The fix is to detect the situation in the instantiator, create an incomplete end to end flow instance, and attach an error to this ete instance.

AaronGreenhouse commented 6 years ago

Still feeling a bit stumped by this, but let me try one more time. I think I'm getting much closer to understand the real issues here.

You are saying the problem is that while generating the end to end flow, the actual connections in the instance model lead us to sub4, and then we don't know what to do.

So I have a few comments about this:

Why is the end to flow generation process immediately following connection conn3 (to sub4) when there isn't a flow path that tells it to? If it didn't do this, the simple flow path af1 -> af2 would be satisfactory. In this case the end to end flow instance would be

and everything would be fine.

Now, if the flow path were af1 -> conn3 -> sub4 -> conn4 -> af2 then you would of course follow conn3, because it is right there in the flow path.

I am still of the opinions that

lwrage commented 6 years ago
AaronGreenhouse commented 6 years ago
AaronGreenhouse commented 6 years ago

Okay, so one more time:

The specific problem here is that the semantic connection we are following in the end-to-end flow instance arrives a subcomponent that is not mentioned in the flow path implementation.

lwrage commented 6 years ago

Conceptually, yes. It's been a while, but think the implementation finds connection instances that are candidates (meaning they reference one (or more) declarative connections) it then checks if the next flow path is connected to the end of one of the candidates. That code doesn't seem to properly handle the case where no candidate matches.

AaronGreenhouse commented 6 years ago

Okay, I'm pretty sure I figured out where/when to create the error for this situation. But when I create a marker on a node in the AADL file from the instanstiator, the marker doesn't get the location set at all. So it's not very useful. Not clear why this is. There are other places in the instantiator where markers are created on the AADL file (and not the instance model). Pretty sure those don't work correctly either.

AaronGreenhouse commented 6 years ago

Error needs to be reported when the isValidContinuation() methods return false. 2 methods here, one for a flow following a connection, and one for a connection following a flow.

AaronGreenhouse commented 6 years ago

I cannot figure out how to trigger the case of the second isValidContinuation() method where it seems to be checking if a flow ends at the start of semantic connection. Seems I would have to create a semantic connection that starts in the middle of the specified flow, but that somehow still bypasses the flow.

lwrage commented 6 years ago

Probably meant to cover a symmetric case to the first one. However, I don't think that case can occur because we collect a list of declarative connections from the flow specs and impls and then match semantic connections that start with this list. (I didn't actually look at the code, but that's how it should be implemented.)

AaronGreenhouse commented 6 years ago

Right. It looks like it is mean to capture the opposite case as the first one, but it's not clear it can happen in practice. I won't worry about testing it for now. I'll make tests for the other cases I can create and then move on.

lwrage commented 6 years ago

I have convinced myself that this opposite cannot happen in practice.

On Tue, Jan 9, 2018 at 11:38 AM, AaronGreenhouse notifications@github.com wrote:

Right. It looks like it is mean to capture the opposite case as the first one, but it's not clear it can happen in practice. I won't worry about testing it for now. I'll make tests for the other cases I can create and then move on.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/osate/osate2-core/issues/879#issuecomment-356339824, or mute the thread https://github.com/notifications/unsubscribe-auth/AAty5-jBuFeh_ib5Axn4rnrqYxkGdAKCks5tI5X5gaJpZM4OxMDp .

AaronGreenhouse commented 6 years ago

Finished JUnit tests

lwrage commented 6 years ago

The following model may trigger the second case as cc1 doesn't occur in any flow specification. This is a special case where the data access is treated as a proxy for the data component. In this case the flow is implicitly extended to the data component (if present). (The error reported on ete is a bug.)

package X
public
    system T
    end T;

    system implementation T.i
        subcomponents
            p: abstract P.i;
            c: abstract C;
        connections
            ct: port p.da -> c.fpi;
        flows
            ete: end to end flow p -> ct -> c;
    end T.i;

    abstract P
        features
            da: provides data access D;
    end P;

    abstract implementation P.i
        subcomponents
            d: data D;
        connections
            cc1: data access da <-> D;
    end P.i;

    data D
    end D;

    abstract C
        features
            fpi: in data port D;
    end C;

end X;
AaronGreenhouse commented 6 years ago

This did not cause any problems. The end to end flow instance is created without any problems.

lwrage commented 6 years ago

It wasn't supposed to. It may be possible to trigger the issue by adding a flow source specification and increasing the nesting, but it's not worth the effort.

On Thu, Jan 11, 2018 at 11:29 AM, AaronGreenhouse notifications@github.com wrote:

This did not cause any problems. The end to end flow instance is created without any problems.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/osate/osate2-core/issues/879#issuecomment-356982536, or mute the thread https://github.com/notifications/unsubscribe-auth/AAty5864jIO2ZldsMDI9wLGxKM79wVd0ks5tJjcDgaJpZM4OxMDp .