Redundant overdetermined2

[HansOlsson]

Closes #3370
I hope this makes it more understandable than the previous attempt.

[henrikt-ma]

I think there's some room for improvement, but overall I like the approach.

[eshmoylova]

I am missing of what is meant by redundant equations. I have spent good two hours trying to figure out how redundant equations are handled in my code and how any of it is based on anything in the specification, only to realize that my definition of redundant is not the same as what is meant in the discussion in #3373. I am not sure if it best to continue this discussion in this PR or if it would be better to return to the issue.

[HansOlsson]

@henrikt-ma @eshmoylova is it ok now?

[eshmoylova]

I see 3 types of redundant equations:

1. All distinct equations, but they form a cycle in the graph
2. Identical equations
3. Connecting the connector to itself as in the first example in #3370.

Handling type 1 is the whole purpose of constructing tree from the graph and is already covered by the specification.

Type 2 is not really handled as far as I can see. In my implementation I store the edges of the graph in a set and it eliminates exactly identical equations. But it is rather an implementation choice and has no basis in the specification. This is the type I would like to see clarified. The subtype of type 2 is the same equation but in different direction connect(A,B); connect(B,A); If direction is distinct, it would produce two edges. One can be added to a tree (assuming no other cycles would exist) and we would have an equality and another one would be removed and an equalityConstraint would be generated. That does not seem correct. From the description of edges of the connection graph it is not clear if the graph is directional.

Type 3 is another type of tricky. The way it was in the example, I got a connection set consisting of just one element and with certain level of checks turned on that produces an error. But if there would be any other elements in the same connection set that would be allowed.

In the current PR I am not seeing how the type 2 and 3 are addressed. My suggestion would be as follows.

1. Modify Definition 9.1 (section 9.2):

The primitive elements are of simple types, overconstrained types, or of types defined as operator record (i.e., a component of an operator record type is not split into sub-components).

2. To the paragraph that follows (section 9.2) add a special handling for overconstained, something like:

For overconstained primitive types, in addition to the connection set, a set of edges is collected based on the used connect equations and other overconstained equation operators as described in section 9.4.1.

I am saying set here, and to me it implies that exactly unique edges would be eliminated. But it may be not what is desired and/or handled by the next suggestion.
3. In section 9.4.1 it should be specified in the description of operators whether connect(A,B) and connect(B,A) form the same edge and whether Connections.branch(A.R,B.R) and Connections.branch(B.R,A.R) produces an error or the same branch.
4. Finally, to address type 3, we need to add somewhere maybe in the description of connect and Connections.branch what happens if the edge is from A to A.

[HansOlsson]

Hmmm.. I see that it is still unclear, and I think we need an example for this

[henrikt-ma]

As the language group seems positive to the general idea according to #3385 (comment), I won't worry about not having test-implemented the idea myself. The text can still be improved a bit, as suggested, but overall I agree that the procedure looks plausible.

[eshmoylova]

Hopefully, we can discuss it during the phone meeting.

[HansOlsson]

I believe it is now ready.
There are two minor items that can be handled in separate PR:

• Updating the figure to use the new notation (added previously), instead of this idea of using the old notation in the figure and explaining it in the caption. @henrikt-ma
• Some rule stating that you shouldn't combine connect and Connections.branch for the same connector. (BTW: This corresponds to the general rule of not mixing graphical (connect) and non-graphical (Connections.branch and equations) in the same model.)

[HansOlsson]

@henrikt-ma can you review this one, or dismiss your review in some way if you don't find it relevant?

[henrikt-ma]

Looks good.

It would look even better with lower case a and b for the connector instances in examples, but this can be improved in a separate PR. Also, I'll take care of updating the illustration separately, as suggested by @HansOlsson.