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

Should we broaden analyser checks for overconstrained connected variables #1162

Open
nickerso opened this issue May 2, 2023 · 3 comments
Open

Comments

@nickerso
Copy link
Contributor

nickerso commented May 2, 2023

          If I have the following model:
<?xml version='1.0' encoding='UTF-8'?>
<model name="BG3" xmlns="http:https://www.cellml.org/cellml/2.0#" xmlns:cellml="http:https://www.cellml.org/cellml/2.0#">
    <units name="m_per_s">
        <unit units="metre"/>
        <unit exponent="-1" units="second"/>
    </units>
    <component name="main">
        <variable name="t" units="second"/>
        <variable initial_value="1" name="v_in" units="m_per_s" interface="public"/>
        <!-- State variables-->
        <variable initial_value="-2.5" name="v" units="m_per_s"/>
        <math xmlns="http:https://www.w3.org/1998/Math/MathML">
            <apply>
                <eq/>
                <apply>
                    <diff/>
                    <bvar>
                        <ci>t</ci>
                    </bvar>
                    <ci>v</ci>
                </apply>
                <ci>v_in</ci>
            </apply>
        </math>
    </component>
    <component name="base">
        <variable initial_value="1" name="v_out" units="m_per_s" interface="public"/>
    </component>
    <connection component_1="main" component_2="base">
        <map_variables variable_1="v_in" variable_2="v_out"/>
    </connection>
</model>

The analyser tells me that

Variable 'v_out' in component 'base' and variable 'v_in' in component 'main' are equivalent and cannot therefore both be initialised.

But they are initialised to the same value so I don't think that this should be an error.

Originally posted by @hsorby in #942 (comment)

@agarny
Copy link
Contributor

agarny commented May 2, 2023

I am not personally in favour of it. This would encourage bad practice.

@nickerso
Copy link
Contributor Author

nickerso commented May 2, 2023

I am not personally in favour of it. This would encourage bad practice.

Its not always bad practice...imagine a model that imports a component with parameter values embedded in the initial values. Might be nice to be able to override them by providing initial values in the importing model. But I actually think this would require a spec change to specify the precedence of initial values so that it is reproducible for a given model what is the initial value for a connected variable set.

@hsorby
Copy link
Contributor

hsorby commented May 2, 2023

My question above is about when the initial value is set to the same value, this to me doesn't make the system overconstrained. Deciding which initial value should be dominant because they are different seems to me to be a separate issue.

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

No branches or pull requests

3 participants