Update bus load analysis internal model to use ecore

AaronGreenhouse commented 3 years ago

After discussion of general analysis implementation issues with @lwrage we decided that the internal models should be based on Ecore for uniformity with the rest of OSATE.

The model used by Bus Load analysis needs to be converted to Ecore.

AaronGreenhouse commented 3 years ago

See https://www.vogella.com/tutorials/EclipseEMF/article.html

AaronGreenhouse commented 3 years ago

Created a new model busload.ecore. This uses a new generic element that created in analysis.ecore. The analysis.ecore model will eventually be moved to a new project/plugin.

Need to create some infrastructure for traversing the EMF model. Trying to think about what Anna is going to need for the other Resource analyses and to figure out what is the generic way of doing things.

AaronGreenhouse commented 3 years ago

Some comments on this:

Mutability

When implementing the analysis model using plain-old Java classes (POJC) I am able to make fields final to better protect them from accidental changes if the class is misused. I'm a big fan of doing this (but I notice that most large APIs from major vendors don't seem to do this). Ecore models make all the attributes publicly settable, and there is no set-once option. This isn't a really big deal, but a downside in my mind.

Model Traversal

For some inexplicable reason, EMF does not provide any way to control the traversal of Ecore models. It does not have a proper visitor pattern. As far as I can tell you are expected to use EcoreUtil.getAllContents to get a TreeIterator and then repeated pass the model objects to a Switch instance. This is a bit clunky, but so be it. The bigger deal is that (1) although getAllContents does a preorder traversal, this is not documented as a requirement, and (2) there is no provision for a postorder traversal.

Busload analysis needs a post order traversal to compute the loads. So I need to make my own machinery to do this. This seems wasteful, but there is no way around it. To this end I have the AnalysisModelTraversal class that is intended to be generic and used by other analysis implementations.

A side-effect of the clunkiness of using a Switch instance instead of a proper visitor pattern, is that it is difficul to maintain state during a traversal. In particular, Busload analysis needs to maintain the current data overhead value and the current Result object during the traversal, and this is computed in a top-down (preorder) manner. In a simple recursive descent implementation of a traversal that would be available in a POJC implementation, this could be maintained by a parameter to the visitation method. But this is not possible when using a Switch class. So instead I have to have an initial preorder traversal of the model and store the overhead and Result values as attributes of the model elements themselves, so they can then be read from the model during the "real analyses" in a second bottom up pass.

This adds unnecessary clunkiness to the implementation, and I would argue unnecessary junk into the analysis model.

Switch return values

The Switch class uses caseXXX methods to handle each EClass declared in the EMF EPackage. If a method returns null (the default implementation of each method), then the case method for the superclass of the EClass is called, and so on. The catch here is that if you have a Switch representing an operation that doesn't have a return value, you should not paratemerize the Switch with a Void class. This class has no instances, and so null is the only legal return value that can be used with it. This would cause the superclass case methods to always be invoked, all the way up the hierarchy. Not good. To get around this, I have instead made the return type Boolean and I return the value Boolean.TRUE if the case is handled.

A cleaner approach, I supposed, would be to introduce a new enumeration:

enum SwitchVoid {
    VOID;
]

and return SwitchVoid.VOID when successfully handled, and null otherwise. This would avoid assigning unwanted semantics to Boolean values.

AaronGreenhouse commented 3 years ago

@lwrage Pointed out that the above comment sounds like I was against using EMF. I was just trying to document how it forces a different way of doing things. Of course, the advantage of using EMF, is it provides uniformity across the analyses and with the main AADL models.

I haven't tested the my EMF version of the bus load analysis yet. Should be able to do so shortly.

My main complaint against EMF is with the model traversal and Switch classes. I used them because this seems to be the way that they advocate that you do it. But, well, it has problems. The traversal I wrote in the non-EMF version isn't perfect either. It uses an explicit stack to manage the values that are "passed down" during the pre-order traversal. This is not ideal either. This comes from the fact that that approach is visitor-based, even though the objects themselves are controlling the traversal.

So after I finish testing the new EMF version, I am going to make a third version, that also uses EMF, but instead of using visitors or Switch classes, features a straightforward recursive traversal of the tree (with the operation defined as part of the model) that allows for parameters to be directly passed so we can exploit the method-call stack.

Then we will have 3 implementations of the same analysis and be able to make some informed decisions about how to proceed in general.

AaronGreenhouse commented 3 years ago

More notes:

Forgot to make the references "containment".
I had made BusloadElement an unrelated to class to AnalysisElement and then used multiple inheritance. The BusLoadModel was an AnalysisElement but not a BusloadElement. This caused problems with the implementation of the isLeaf() and getOrderedChildren() operations because then the implementations from AnalysisElement weren't inherited and I would have to re-implement the methods in each class, which was undesirable. So I had to make BusLoadElement a subclass of AnalysisElement, and BusLoadModel a subclass of BusLoadElement.

AaronGreenhouse commented 3 years ago

Had to fiddle with the order of the reference declarations in BusOrVirtualBus to make them consistent with the order expected by the results. (That is to keep things consistent with the original version of the analysis.). Wasn't sure how to control this in the graphical editor, so I had to edit the XML directly.

AaronGreenhouse commented 3 years ago

Reg tests now pass. I didn't any actual coding errors (that I have discovered yet). All the problems were from EMF meta modeling. :-(

AaronGreenhouse commented 3 years ago

Okay, so I have the first working version: Ecore/EMF using iteration and Switch classes. This is on branch 2555_bus_load_analysis_ECORE

AaronGreenhouse commented 3 years ago

Going to make 2 more versions:

An EMF version that does not use Switch, but instead has explicit recursive traversal methods.
An improved POJO version that has explicit recursive traversal methods.

AaronGreenhouse commented 3 years ago

Created branch 2555_bus_load_analysis_POJO_no_visitor for the second choice above.

AaronGreenhouse commented 3 years ago

Created alternate version of POJO traversal. No visitor. Instead each model class has an analyzeXXX() method. The visitBroadcast() method from Broadcast shows some of tradeoffs involved:

(Branch 2555_bus_load_analysis_POJO_no_visitor.)

    public final void analyzeBroadcast(final Result broadcastResult, final double dataOverheadKBytes) {
        connections.forEach(connection -> connection.analyzeConnection(connection.createAndAddResult(broadcastResult),
                dataOverheadKBytes));

        // Compute the actual usage and budget requirements
        final double actual = getConnectionActualKBytesps(getSource(), dataOverheadKBytes);
        setActual(actual);

        // Use the maximum budget from the connections, warn if they are not equal
        double maxBudget = 0.0;
        boolean unequal = false;
        Connection last = null;
        for (final Connection c : getConnections()) {
            final double current = c.getBudget();
            maxBudget = Math.max(maxBudget, current);
            if (last != null) {
                unequal = last.getBudget() != current;
            }
            last = c;
        }
        setBudget(maxBudget);

        ResultUtil.addRealValue(broadcastResult, maxBudget);
        ResultUtil.addRealValue(broadcastResult, actual);

        if (unequal) {
            for (final Connection c : getConnections()) {
                warning(broadcastResult, c.getConnectionInstance(),
                        "Connection " + c.getConnectionInstance().getName() + " sharing broadcast source "
                                + getSource().getInstanceObjectPath() + " has budget " + c.getBudget()
                                + " KB/s; using maximum");
            }
        }
    }

Advantages:

The parent Result object and the current data overhead value are taken as method parameters. The current result and updated overhead value are passed to the analyzeConnection() method of the child Connection nodes.
Prefix and Postfix activities are in the same place.

Disadvantages

Analysis code is now spread over the classes Broadcast, Connection, and BusOrVirtualBus.
Where to put analysis helper methods? The methods getConnectionActualKBytesps() (called from the method above), warning() and error() have been put into the root AnalysisElement class. This is not ideal. Of course they could be put in a utilities class or other place, but the point is they too are separated from the rest of the analysis code.

Other comments

Using this traversal approach with Ecore/EMF would have the same problems.

Realized when working on the next version (2555_bus_load_analysis_POJO_visitor_state) that I messed up the state being passed to the analysisXXX methods: I the incoming overhead value was immediately applicable in the context of the analyzed element. But the incoming Result was the result for the parent (it was even called parentResult) and the Result for the current model element needed to be created. This is wrong. All the incoming state should be immediately applicable to the element in the analyzeXXX method. The root of this problem could be seen in my analyzeBusLoadModel() method. This method took the Result for the specific system operation mode as input, and passed it directly to the child bus elements. This didn't make any sense, two layers of the model were being passed the same Result object.

Had to add createAndAddResult() and the abstract createResult() methods to the abstract class AnalysisElement. You can see how this is used in analyzeBroadcast() above:

        connections.forEach(connection -> connection.analyzeConnection(connection.createAndAddResult(broadcastResult),
                dataOverheadKBytes));

This raises interesting semantic problems below for 2555_bus_load_analysis_POJO_visitor_state.

AaronGreenhouse commented 3 years ago

Going to try a new approach: Visitor with pre and postfix method where the visit methods take a state parameter.

I think this may be the best of both worlds, and is extendable to use with Ecore/EMF.

This will be on branch 2555_bus_load_analysis_POJO_visitor_state

AaronGreenhouse commented 3 years ago

Comments on version in 2555_bus_load_analysis_POJO_visitor_state

This turned out to be harder to do than I thought. This is because Java's generic types don't work out nicely for it. To be completely type safe we need to declare the types using Generic Self types, which are not supported by Java. They can be approximated, but it causes a lot of problems. More details on this below when I discuss the realized implementation. So this version has to make use of some type casting, but most of it is relegated to the "boiler plate" template code used to callback to the model visitor.

The point of this version was to try to move all the analysis code back into a single visitor class that handles both the prefix and postfix traversal of the model, this is in contradistinction to

The first version that has two visitors, one for prefix and one for postfix
The second version that has no visitor, but distributes the analysis code across all the model elements.

I was able to do this successfully, but the abstraction is a little more complicated than just "handle the prefix case" and "handle the postfix case". As described in a previous comment, I realized that I had been passing down the Result object incorrectly: each node was receiving the Result for its parent, and then creating a new Result object for itself. This is not right. Each node should have been receiving the Result object for itself, just like it is receiving the data overhead value that is applicable to itself. This means that the Result object needs to be created during the visit of the parent node. This tricky because the actual traversal of the tree is being controlled external to the visitor by the visit() method in the model class ModelElement:

public abstract class ModelElement {
    /*
     * Here we put the state before the visitor to make the initial state easier to find in the initial
     * method call. This is assuming the we have something like
     *
     * rootNode.visit(<initial state>, new Visitor<..> { ... });
     */
    public final <S> void visit(final S state, final Visitor<S> visitor) {
        final Primed<S> sv = visitSelfPrefix(visitor, state);
        visitChildren(visitor, sv.state, sv.transformer);
        visitSelfPostfix(visitor, sv.state);
    }

    final <S> void visit(final List<? extends ModelElement> children, final Visitor<S> visitor, final S state,
            final StateTransformer<S> transformer) {
        children.forEach(e -> e.visit(transformer.transformState(state, e), visitor));
    }

    abstract <S> Primed<S> visitSelfPrefix(Visitor<S> visitor, S state);

    abstract <S> void visitChildren(Visitor<S> visitor, S state, StateTransformer<S> transformer);

    abstract <S> void visitSelfPostfix(Visitor<S> visitor, S state);
}

So what we actually have are three phases to a node:

The visitPrefix() method is called, which generally forwards the call to the appropriate method in the visitor, e.g.,

public final class Broadcast extends AnalysisElement {
    // . . .
    @ Override
    <S> Primed<S> visitSelfPrefix(Visitor<S> visitor, S state) {
        return ((BusLoadVisitor<S>) visitor).visitBroadcastPrefix(this, state);
    }
    // . . .
}

Here we see the first place a cast is required. The visitor needs to be cast to the specific type of visitor expected for the overall model. This required in the implementation of visitSelfPostfix() as well.

The semantics of this method are that any state updates that do no depend on the specifics of a child node are made and returned. (How? See below.).

The second part is that each child node is visited, and provided with a state object specific to that node. This is motivated by the need to create a Result object specific to each child node as mentioned above. Because the actual traversal of the child nodes is elsewhere, we need a way to control what is given to a child node that can be run elsewhere. Thus the prefix method needs to also return a lambda expression that controls this.

So, the prefix method needs to return a state and a lamda. This is done by returning a Primed object (as in "state primed" or S') that is a pair of the state and the lambda. Each child of the node is passed to the lambda along with the primed state to obtaine the state that is used when visiting the child node.

An example of how this works in practice is the prefix method for VirtualBusOrBus:

    private final static StateTransformer<BusLoadState> CREATE_RESULT_FOR_CHILD = (state, child) -> state
            .setResult(((ResultElement) child).makeResult(state.result));
// ...
        @Override
        public Primed<BusLoadState> visitBusOrVirtualBusPrefix(final BusOrVirtualBus bus, final BusLoadState inState) {
            final ComponentInstance busInstance = bus.getBusInstance();
            final double localOverheadKBytesps = GetProperties.getDataSize(busInstance, GetProperties.getKBUnitLiteral(busInstance));
            return Primed.val(inState.increaseOverhead(localOverheadKBytesps), CREATE_RESULT_FOR_CHILD);
        }

First the data overhead is updated by getting the local overhead contribution. It is added to the existing overhead (the call to inState.increateOverhead()) to get the intermediate state that each child's state is derived from. The constant CREATE_RESULT_FOR_CHILD is a lambda (of class StateTransformer) that further updates the state by replacing the Result object with one appropriate for the given child object.

These are bundled up into a single Primed object and returned.

Finally the postfix visit is performed by calling visitSelfPostfix(). Again this is generally implemented to call into the visitor:

public final class Broadcast extends AnalysisElement {
    // ...

    @Override
    <S> void visitChildren(Visitor<S> visitor, S state, StateTransformer<S> transformer) {
        visit(connections, visitor, state, transformer);
    }

    @Override
    <S> void visitSelfPostfix(Visitor<S> visitor, S state) {
        ((BusLoadVisitor<S>) visitor).visitBroadcastPostfix(this, state);
    }

    @Override
    public Result makeResult(final Result parentResult) {
        final Result broadcastResult = ResultUtil.createResult("Broadcast from " + source.getInstanceObjectPath(),
                source, ResultType.SUCCESS);
        return addResultToParent(parentResult, broadcastResult);
    }
}

Here we also see that each node is also expected to implement a visitChildren() method which is also a boiler plate method that calls an inherited visit() method with list of children. We also see the specifics of an implementation of makeResult() (used by the CREATE_RESULT_FOR_CHILD lambda expression).

Overall, the results are successful in that the analysis can be expressed in a single visitor:

    private final class BusLoadAnalysisVisitor implements BusLoadVisitor<BusLoadState> {
        @Override
        public Primed<BusLoadState> visitBusLoadModelPrefix(final BusLoadModel model, final BusLoadState inState) {
            return Primed.val(inState, CREATE_RESULT_FOR_CHILD);
        }

        @Override
        public Primed<BusLoadState> visitBusOrVirtualBusPrefix(final BusOrVirtualBus bus, final BusLoadState inState) {
            final ComponentInstance busInstance = bus.getBusInstance();
            final double localOverheadKBytesps = GetProperties.getDataSize(busInstance, GetProperties.getKBUnitLiteral(busInstance));
            return Primed.val(inState.increaseOverhead(localOverheadKBytesps), CREATE_RESULT_FOR_CHILD);
        }

        @Override
        public void visitBusOrVirtualBusPostfix(final BusOrVirtualBus bus, final BusLoadState state) {
            final long myDataOverheadInBytes = (long) (1000.0 * state.dataOverheadKBytesps);

            // Compute the actual usage and budget requirements
            double actual = 0.0;
            double totalBudget = 0.0;

            for (final BusOrVirtualBus b : bus.getBoundBuses()) {
                actual += b.getActual();
                totalBudget += b.getBudget();
            }
            for (final Connection c : bus.getBoundConnections()) {
                actual += c.getActual();
                totalBudget += c.getBudget();
            }
            for (final Broadcast b : bus.getBoundBroadcasts()) {
                actual += b.getActual();
                totalBudget += b.getBudget();
            }
            bus.setActual(actual);
            bus.setTotalBudget(totalBudget);

            final ComponentInstance busInstance = bus.getBusInstance();
            final double capacity = GetProperties.getBandWidthCapacityInKBytesps(busInstance, 0.0);
            final double budget = GetProperties.getBandWidthBudgetInKBytesps(busInstance, 0.0);
            bus.setCapacity(capacity);
            bus.setBudget(budget);

            final Result busResult = state.result;
            ResultUtil.addRealValue(busResult, capacity);
            ResultUtil.addRealValue(busResult, budget);
            ResultUtil.addRealValue(busResult, totalBudget);
            ResultUtil.addRealValue(busResult, actual);
            ResultUtil.addIntegerValue(busResult, bus.getBoundBuses().size());
            ResultUtil.addIntegerValue(busResult, bus.getBoundConnections().size());
            ResultUtil.addIntegerValue(busResult, bus.getBoundBroadcasts().size());
            ResultUtil.addIntegerValue(busResult, myDataOverheadInBytes);

            if (capacity == 0.0) {
                warning(busResult, busInstance, (bus instanceof Bus ? "Bus " : "Virtual bus ") +
                        busInstance.getName() + " has no capacity");
            } else {
                if (actual > capacity) {
                    error(busResult, busInstance,
                            (bus instanceof Bus ? "Bus " : "Virtual bus ") + busInstance.getName()
                            + " -- Actual bandwidth > capacity: " + actual + " KB/s > "
                            + capacity + " KB/s");
                }
            }

            if (budget == 0.0) {
                warning(busResult, busInstance, (bus instanceof Bus ? "Bus " : "Virtual bus ") +
                        busInstance.getName() + " has no bandwidth budget");
            } else {
                if (budget > capacity) {
                    error(busResult, busInstance,
                            (bus instanceof Bus ? "Bus " : "Virtual bus ") + busInstance.getName()
                                    + " -- budget > capacity: " + budget + " KB/s > " + capacity + " KB/s");
                }
                if (totalBudget > budget) {
                    error(busResult, busInstance,
                            (bus instanceof Bus ? "Bus " : "Virtual bus ") + busInstance.getName()
                            + " -- Required budget > budget: " + totalBudget + " KB/s > " + budget
                            + " KB/s");
                }
            }
        }

        @Override
        public Primed<BusLoadState> visitBroadcastPrefix(final Broadcast broadcast,
                final BusLoadState inState) {
            return Primed.val(inState, CREATE_RESULT_FOR_CHILD);
        }

        @Override
        public void visitBroadcastPostfix(final Broadcast broadcast, final BusLoadState state) {
            // Compute the actual usage and budget requirements
            final double actual = getConnectionActualKBytesps(broadcast.getSource(), state.dataOverheadKBytesps);
            broadcast.setActual(actual);

            // Use the maximum budget from the connections, warn if they are not equal
            double maxBudget = 0.0;
            boolean unequal = false;
            Connection last = null;
            for (final Connection c : broadcast.getConnections()) {
                final double current = c.getBudget();
                maxBudget = Math.max(maxBudget, current);
                if (last != null) {
                    unequal = last.getBudget() != current;
                }
                last = c;
            }
            broadcast.setBudget(maxBudget);

            final Result broadcastResult = state.result;
            ResultUtil.addRealValue(broadcastResult, maxBudget);
            ResultUtil.addRealValue(broadcastResult, actual);

            if (unequal) {
                for (final Connection c : broadcast.getConnections()) {
                    warning(broadcastResult, c.getConnectionInstance(),
                            "Connection " + c.getConnectionInstance().getName() + " sharing broadcast source "
                                    + broadcast.getSource().getInstanceObjectPath() + " has budget " + c.getBudget()
                                    + " KB/s; using maximum");
                }
            }
        }

        @Override
        public void visitConnectionPostfix(final Connection connection, final BusLoadState state) {
            final ConnectionInstance connectionInstance = connection.getConnectionInstance();
            final double actual = getConnectionActualKBytesps(connectionInstance.getSource(),
                    state.dataOverheadKBytesps);
            connection.setActual(actual);

            final double budget = GetProperties.getBandWidthBudgetInKBytesps(connectionInstance, 0.0);
            connection.setBudget(budget);

            final Result connectionResult = state.result;
            ResultUtil.addRealValue(connectionResult, budget);
            ResultUtil.addRealValue(connectionResult, actual);

            if (budget > 0.0) {
                if (actual > budget) {
                    error(connectionResult, connectionInstance, "Connection " + connectionInstance.getName()
                            + " -- Actual bandwidth > budget: " + actual + " KB/s > " + budget + " KB/s");
                }
            } else {
                warning(connectionResult, connectionInstance,
                        "Connection " + connectionInstance.getName() + " has no bandwidth budget");
            }
        }
    }

Because this visitor is now located separate from the model classes themselves, the helper methods, e.g. error(), can be located with it.

The downside here is the craziness in the prefix methods. This can be avoided if I instead add another abstract method to ModelElement: updateStateForChild(), and this too would call back into the visitor to an updateStateForChildOfXXX() method. At the time I though this might make things too verbose, but that was before I realized I was going to need the Primed class.

AaronGreenhouse commented 3 years ago

Created branch from 2555_bus_load_analysis_POJO_visitor_state — 2555_bus_load_analysis_POJO_visitor_state_no_primed — that will see what it's like to get rid of the Primed class and lambda expression.

AaronGreenhouse commented 3 years ago

Summary of different version of the BusLoad Analysis:

Branch master — implemented with POJO using a visitor pattern. No explicit distinction between pre- and postfix traversal. The visitor handles the stack explicitly.
Branch 2555_bus_load_analysis_ECORE — implemented with Ecore/EMF using a visitor pattern. Has explicit prefix and postfix traversal methods. Must make two passes (prefix and postfix), so has two visitors.
Branch 2555_bus_load_analysis_POJO_no_visitor — Implemented with POJO and no visitor. Model objects have explicit analyze methods that are free to visit child nodes in what ever order they like.
Branch 2555_bus_load_analysis_POJO_visitor_state— Implemented with POJO and a visitor, but that visitor is able to distinguish between pre and postfix visitation and the visits methods are passed state objects.
Branch 2555_bus_load_analysis_POJO_visitor_state_no_primed — A slightly different version of 2555_bus_load_analysis_POJO_visitor_state that uses an abstract method in place of a lambda expression.

AaronGreenhouse commented 3 years ago

For reasons that will detailed later, it seems the 2555_bus_load_analysis_ECORE branch is closest to what we should do. The remaining problem with it is that any prefix order related state needs to be put in the model. More specifically, the model needs to be designed ahead of time to store this state. This is no good, especially if there is more than one prefix traversal that needs to be made. Furthermore these features are always there. Even when they aren't currently needed. It would be better if they were more dynamic.

An example of where this is a problem is with BusLoadModel.print(), which I did not move to the ECORE version. To do so in same manner as I updated the analysis itself would require adding an "indent" attribute to the model, which seems silly.

Going to try one final version where I use EMF adapters to handle the state used visitation.

Started branch 2555_bus_load_analysis_ECORE_adapters from branch 2555_bus_load_analysis_ECORE.

AaronGreenhouse commented 3 years ago

Actually, not going to use the EMF Adapters because they are too heavy weight. In particular, I don't care about the notification aspect of them. Even worse, because they become connected to the original EObject, they don't go away when you are done with them. Furthermore, there is only one of each type of adapter made for a node. While unlikely, this would cause problems if say print() was called from two different threads at the same time.

For my purposes, I think it is best just to use a regular Java Map between EObjects and data.

AaronGreenhouse commented 3 years ago

Question: What should be and should not be attributes in the analysis model?

Do we need the Result object to be an attribute? Not sure it has much value there because it is reachable from the root AnalysisResult object.
What about the data overhead? We need to to compute the overhead to find the actual usage.
Do we even need the actual and budget as attributes? These are stored in the Result object. If we put these in the model, then we should put overhead to be consistent.
Why is capacity not in the model? It can be obtained from the property association. Only the local node needs the information; it is not used to compute a value for any other node.
Clearly the indent value in the print method does not need to be stored in the tree.

I think this decision is a little bit fuzzy, but values that are clearly related to the core purpose of the model, in this case representing the data capacity and usage of the system, should/could be actual attributes in the model. In this case that means overhead, actual, and budget. You could argue that budget doesn't need to be in the model because it can be obtained by a property association just like capacity. But in this case, budget is used both by the local node and by its parent node. Storing it saved the trouble of looking it up a second time. I could see not including budget as an attribute, but I think it should remain to avoid the ugliness of looking up the property association twice, which is expensive.

I don't see a good reason for the Result object to be in the model.

AaronGreenhouse commented 3 years ago

Created wiki page: https://wiki-int.sei.cmu.edu/confluence/display/AADL/Analysis+Model+Implementation+Notes

AaronGreenhouse commented 3 years ago

After much experimentation as detailed in the above wiki page, I have decided that using ECORE EMF is the best approach. I have cleaned up the branch 2555_bus_load_analysis_ECORE_adapters by moving the analysis.ecore model and the AnalysisModelTraversal class to a new project org.osate.model.analysis. Removed the old model classes.

AaronGreenhouse commented 3 years ago

Created a tag 2555_bus_load_analysis_POJO_visitor on the master branch to save the original version of the new bus load analysis that uses POJO with a visitor.

Summary of different version of the BusLoad Analysis:

Branch 2555_bus_load_analysis_POJO_visitor — implemented with POJO using a visitor pattern. No explicit distinction between pre- and postfix traversal. The visitor handles the stack explicitly.
Branch 2555_bus_load_analysis_ECORE — implemented with Ecore/EMF using a visitor pattern. Has explicit prefix and postfix traversal methods. Must make two passes (prefix and postfix), so has two visitors.
Branch 2555_bus_load_analysis_POJO_no_visitor — Implemented with POJO and no visitor. Model objects have explicit analyze methods that are free to visit child nodes in what ever order they like.
Branch 2555_bus_load_analysis_POJO_visitor_state— Implemented with POJO and a visitor, but that visitor is able to distinguish between pre and postfix visitation and the visits methods are passed state objects.
Branch 2555_bus_load_analysis_POJO_visitor_state_no_primed — A slightly different version of 2555_bus_load_analysis_POJO_visitor_state that uses an abstract method in place of a lambda expression.
Branch 2555_bus_load_analysis_ECORE_adapters — branch from 2555_bus_load_analysis_ECORE that uses hash maps handle the Result object state instead of EMF model attributes. This is version that has been submitted to be merged back into master.

AaronGreenhouse commented 3 years ago

Set the "Update Classpath" property to "false" on busload.genmodel. This keeps it from fiddling with the MANIFEST.MF file. Normally it makes sure that the packages that contain the model classes are exported by the model and that any dependencies are also reexported. But I don't want to do that here because the packages are .internel.

lwrage commented 3 years ago

Closed by merging PR #2572 (issue wasn't linked to PR).

osate / osate2