Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modelica.Electrical.Batteries.Utilities.BusTranscription does not respect the specification around expandable connectors #3147

Closed
qlambert-pro opened this issue Apr 1, 2022 · 7 comments · Fixed by #3241
Labels
enhancement New feature or request
Milestone

Comments

@qlambert-pro
Copy link
Collaborator

qlambert-pro commented Apr 1, 2022

In Modelica.Electrical.Batteries.Utilities.BusTranscription the connect equation

connect(gainLosses.u, stackBus.cellBus.lossPower);

fails to respect

"One connector in the connect equation must reference a declared component, and if the other connector is an undeclared element in a declared expandable connector it is handled as follows (elements that are only potentially present are not seen as declared)"

as lossPower is an undeclared element in undeclared expandable connector cellBus

This relates to #428 and #2172

@HansOlsson
Copy link
Collaborator

For #2172 the main issue was lack of realistic examples, and it now seems we have them.
In particular the array declaration of cellBus is good since it illustrates why following are good ideas:

  • Requiring that it is potentially present; as we otherwise wouldn't know if it should be an array of lossPower (as in stackBusArrays) or array of cellBus (as in stackBus).
  • Should be able to connect to it when only potentially present.
  • Have the feature at all since otherwise we would need a free-floating stack-bus-array-declaration in the model, which seems odd.

@HansOlsson HansOlsson added this to the Phone 2022-2 milestone Apr 26, 2022
@HansOlsson HansOlsson added the enhancement New feature or request label Apr 26, 2022
@HansOlsson
Copy link
Collaborator

HansOlsson commented Jul 1, 2022

Proposed new text:

At least one connector in the connect equation must reference a declared or potentially present component.
If potentially present component it will be marked as being present (recursively if needed).
If the other connector is not even potentially present it must be an undeclared element in a at least potentially present expandable connector and it is handled as follows (after marking the potentially present expandable connector as present):

Non-normative:
The undeclared component in 'potentially present expandable connector' makes it possible to have a 'potentially present array of expandable connectors'.
(Add example.)

Poll using emojis:

  • Thumb up if in favor
  • Thumb down if against
  • Eyes if we need to discuss it more.

@qlambert-pro
Copy link
Collaborator Author

I don't understand the wording. Can you explain what your text is trying to do that specifying that potentially present component are considered declared does not?

@HansOlsson
Copy link
Collaborator

HansOlsson commented Jul 7, 2022

I don't understand the wording. Can you explain what your text is trying to do that specifying that potentially present component are considered declared does not?

I see at least the following reasons:

  • Allow us to deduce their causality - that may differ from their declared causality. Obviously we could have some other variant for handling that.
  • Handle flow-variables in them (if there are any physical connectors inside the expandable connector). The issue is that any flow-variables in expandable connectors need to be added as both inside and outside connectors for the set-handling (otherwise there are too few equations). Adding them even if there aren't any connections seems to make that under-determined in some cases - I have reproduced that below.
  • Making it clear that it behaves as if they aren't there.

I'm sure there might be other solutions, but that might require a lot of work and it is not clear that those potential solutions would be easier to understand.

In the package below MOk doesn't have the potentially present variable and thus translates ok. In MBad I added a silly connections to create it (which shouldn't be legal), and that fails. (The reason for the current rules is if a sub-component has an expandable connector without any contents and it is added elsewhere there would otherwise be a missing equation - I could add that as well.)

package P
  expandable connector C
    annotation (Icon(coordinateSystem(preserveAspectRatio=false)), Diagram(
          coordinateSystem(preserveAspectRatio=false)));
  end C;

  expandable connector SubC
    Modelica.Electrical.Analog.Interfaces.Pin pin;
    annotation (Icon(coordinateSystem(preserveAspectRatio=false)), Diagram(
          coordinateSystem(preserveAspectRatio=false)));
  end SubC;

  model MOk
    C c annotation (Placement(transformation(extent={{-110,-18},{-90,2}})));
    SubC subC annotation (Placement(transformation(extent={{94,26},{114,46}})));
  equation 
    connect(c, subC.sub) annotation (Line(points={{-100,-8},{60,-8},{60,36},{104,
            36}},
          color={0,0,0}), Text(
        string="%second",
        index=-1,
        extent={{-6,3},{-6,3}},
        horizontalAlignment=TextAlignment.Right));
    annotation (Icon(coordinateSystem(preserveAspectRatio=false)), Diagram(
          coordinateSystem(preserveAspectRatio=false)));
  end MOk;

  model MBad
    C c annotation (Placement(transformation(extent={{-110,-18},{-90,2}})));
    SubC subC annotation (Placement(transformation(extent={{94,26},{114,46}})));
  equation 
    connect(c, subC.sub) annotation (Line(points={{-100,-8},{60,-8},{60,36},{104,
            36}},
          color={0,0,0}), Text(
        string="%second",
        index=-1,
        extent={{-6,3},{-6,3}},
        horizontalAlignment=TextAlignment.Right));
    connect(subC.pin, subC.pin) annotation (Line(points={{104.05,36.05},{104.05,10},{90,10},{90,36.05},
            {104.05,36.05}},
          color={0,0,0}), Text(
        string="%first",
        index=-1,
        extent={{-3,6},{-3,6}},
        horizontalAlignment=TextAlignment.Right));
    annotation (Icon(coordinateSystem(preserveAspectRatio=false)), Diagram(
          coordinateSystem(preserveAspectRatio=false)));
  end MBad;
  annotation (uses(Modelica(version="4.0.0")));
end P;

@HansOlsson
Copy link
Collaborator

However, an alternative might be the following:

  1. If a connect-statement references a potentially present component (either as the complete argument or as part of the argument) it will be marked as being present and will be treated as declared below.
  2. After that at least one connector in the connect equation must reference a declared component.
  3. If the other connector is not potentially present it must be in a declared component and is handled as follows:

So for:
connect(gainLosses.u, stackBus.cellBus.lossPower);
stackBus.cellBus is potentially present and is by 1 transformed into declared
Then gainLosses.u is declared (assuming lossPower isn't), so 2 is ok.
And then since lossPower isn't declared we continue with 3.

But we still have the benefit that if we just drag in an expandable connector and don't connect it we avoid all of the issues, both the problem above for physical connectors, and also the lack of source for a causal one.

@qlambert-pro
Copy link
Collaborator Author

qlambert-pro commented Jul 18, 2022

I prefer this alternative but I don't understand the purpose of 3.. There are no potentially present connector then, since 1. made them into declared component. To me 3. sounds like it forbids adding an undeclared signal to an expandable connector. Unless what you mean is:

If the other connector is undeclared it must be a connector inside a declared expandable connector.

@HansOlsson HansOlsson modified the milestones: Phone 2022-2, Phone 2022-4 Sep 2, 2022
@HansOlsson
Copy link
Collaborator

I prefer this alternative but I don't understand the purpose of 3.. There are no potentially present connector then, since 1. made them into declared component. To me 3. sounds like it forbids adding an undeclared signal to an expandable connector. Unless what you mean is:

If the other connector is undeclared it must be a connector inside a declared expandable connector.

Yes, that was the idea - I wrote 'not potentially present' intending to mean 'not even potentially present'; but you are right that undeclared is clearer.

HansOlsson added a commit to HansOlsson/ModelicaSpecification that referenced this issue Sep 15, 2022
Specifically this allows hieararchical additions in buses
 - if the penultimate component is potentially present.
Closes modelica#3147
HansOlsson added a commit that referenced this issue Jan 31, 2023
* More advanced elaboration.
Specifically this allows hieararchical additions in buses
 - if the penultimate component is potentially present.
Closes #3147
* Apply suggestions from code review

Co-authored-by: Henrik Tidefelt <henrikt@wolfram.com>
Co-authored-by: Elena Shmoylova <eshmoylova@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants