Skip to content

Remove causal prefixes when using buses implemented as expandable connectors. - #1499

Closed
qlambert-pro wants to merge 1 commit into
open-ideas:masterfrom
qlambert-pro:master
Closed

qlambert-pro wants to merge 1 commit into
open-ideas:masterfrom
qlambert-pro:master

Conversation

@qlambert-pro

@qlambert-pro qlambert-pro commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Section 9.3 of the specs says:

The matched primitive components of the two connectors must have the same primitive types,
and flow variables may only connect to other flow variables, stream variables only to other stream
variables, and causal variables (input/output) only to causal variables (input/output).

expandable connectors have an inference mechanism to sort out the causality of signals contained in them. Adding a causal prefix makes it invalid to connect to the bus present in
IDEAS.BoundaryConditions.SimInfoManager, for instance, as this one doesn't have such a prefix.
This specific issue was originally discovered in
IDEAS.BoundaryConditions.SolarIrradiation.RadSolData at line 176.

This commit removes all prefixes use for instances of IDEAS.Buildings.Components.Interfaces.WeaBus, an alternative to this PR would be to carefully study how the class is used and to add the appropriate causal prefixes where they are needed.

I can't find a way for this commit to introduce regressions, but somehow my gut is still worried that it might, I just figured I should disclose it, as I wasn't able to fully test the outcome of the change.

…nectors.

`expandable connector`s have an inference mechanism to sort out the
causality of signals contained in them. Adding a causal prefix makes it
invalid to connect to the bus present in
`IDEAS.BoundaryConditions.SimInfoManager`, for instance, as this one
doesn't have such a prefix.
This specific issue was originally discovered in
`IDEAS.BoundaryConditions.SolarIrradiation.RadSolData` at line 176.
@lucasverleyen

lucasverleyen commented Aug 4, 2026

Copy link
Copy Markdown
Member

Dear @qlambert-pro
Thank you very much for identifying this inconsistency. We highly appreciate your valuable input.

I am happy to implement your suggested changes, as I could not identify any potential regressions. The conclusions of my quick impact assessment:

Your suggestions involve the following two models:

  1. IDEAS/BoundaryConditions/SolarIrradiation/RadSolData.mo
    This model is used by four other models:
    -- IDEAS/Buildings/Components/OuterWall.mo
    This model is used extensively throughout the library and is covered by many unit tests. None of these tests fail, and I also manually verified the examples included in the library.
    -- IDEAS/Buildings/Components/Window.mo
    This model is used extensively throughout the library and is covered by many unit tests. None of these tests fail, and I also manually verified the examples included in the library.
    -- IDEAS/Buildings/Components/Shading/ShadingControlInclined.mo
    This model is not used elsewhere in the library.
    -- IDEAS/Experimental/Electric/Photovoltaics/Components/PvArray.mo
    This model belongs to an outdated experimental package. Its examples are currently broken independently of your proposed changes, so they are outside the scope of this PR.

  2. IDEAS/LIDEAS/Components/LinearisationInterface.mo
    This model is not used elsewhere in the library.

Based on this assessment, I am confident that the proposed changes are safe to merge.

I have opened a new issue (see #1500) to implement these changes. Once the corresponding PR (#1501) has been merged, I will close this PR.

@lucasverleyen

Copy link
Copy Markdown
Member

Closed by #1501

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants