Skip to content
This repository has been archived by the owner on May 18, 2019. It is now read-only.

Prefer tearing variables with start value for initialization #3079

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lochel
Copy link
Member

@lochel lochel commented May 7, 2019

Related Issues
ticket:5458

Purpose
Prefer tearing variables with start value for initialization.

@lochel
Copy link
Member Author

lochel commented May 7, 2019

@casella @ptaeuber I couldn't remember the discussion on #1668. How should we deal with it?

@OpenModelica-Hudson
Copy link
Member

The test suite is unstable according to OpenModelica_TEST_PULL_REQUEST 2019-05-07_14-31-46.

@lochel
Copy link
Member Author

lochel commented May 7, 2019

There are two models in the testsuite that give bad results:

  • simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestStodolaTurbine.mos
  • simulation/Modelica/inlineFunction/inlineArray1.mos

@casella
Copy link

casella commented May 7, 2019

@casella @ptaeuber I couldn't remember the discussion on #1668. How should we deal with it?

As I understand, the discussion there refers to variables that have some non-zero start attribute, no matter how it is defined.

The requirement here is different: I would try to favour the choice of those variables who have a start attribute explicitly set in the model (or in one of its ancestor) by modifiers, as in the case of the electrical model mentioned in the ticket, see the source code. The rationale is that if the modeller adds such a modifier explicitly, it is because he/she wants to influence the convergence, so ignoring it in general is not a good idea, as this specific case clearly demonstrates.

@casella
Copy link

casella commented May 8, 2019

@lochel

There are two models in the testsuite that give bad results:

* simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestStodolaTurbine.mos

* simulation/Modelica/inlineFunction/inlineArray1.mos

I checked the Hudson report, there are 32 failure cases. Do you mean these two are the ones where wrong simulation results are produced, while all the other ones are just different diagnostic outputs?

@lochel
Copy link
Member Author

lochel commented May 9, 2019

Exactly. Most of the test do no longer show the warning about iteration variables without start values. It is only the two tests which produce bad results:

  • inlineArray1.mos is not compiling anymore
  • ThermoSysPro.Examples.SimpleExamples.TestStodolaTurbine.mos shows now the following warning but simulates fine otherwise:
    The Tearing heuristic has chosen variables with annotation attribute 'tearingSelect = avoid'. Use -d=tearingdump and -d=tearingdumpV for more information.

@casella
Copy link

casella commented May 10, 2019

Exactly. Most of the test do no longer show the warning about iteration variables without start values.

Well, this seems in fact a very positive outcome, isn't it? 😄

It is only the two tests which produce bad results:

* inlineArray1.mos is not compiling anymore

I'll have a look at it ASAP.

* ThermoSysPro.Examples.SimpleExamples.TestStodolaTurbine.mos shows now the following warning but simulates fine otherwise:
  The Tearing heuristic has chosen variables with annotation attribute 'tearingSelect = avoid'. 
  Use -d=tearingdump and -d=tearingdumpV for more information.

This attribute is not coming from the source code, but was added by some OMC backend step. I have currently no idea why OMC would rather avoid using a tearing variable that the modeller gave a start value, but I will check that as well.

@casella
Copy link

casella commented May 13, 2019

* inlineArray1.mos is not compiling anymore

I had a closer look at the BLT this test case. Vector z is only function of time, so it can be computed right away. a[2:5] is a function of z, so it can be computed next. The next strong component has x[1:5] and a[1] as unknowns, and can be solved by using a[1] as tearing variable.

Now, according to the proposal of ticket:5458, since the whole vector a has a local start modifier, all its scalar elements should be preferred in the choice of the tearing variables. (BTW, there is no way to only apply a start value modifier to some elements in an array, unless you introduce additional scalar alias variables, so if we have arrays that's always going to be the case).

However, once the BLT analysis gets to this point, the sub-vector a[2:5] has already been matched to other equations, so a[1] is the only remaining preferred candidate as tearing variable, as well as the best choice, and the one which older versions of OMC would pick.

@lochel I'm not sure why this PR should break such a case, maybe the modified algorithm in this PR tries too hard to select all the variables with start modifiers in the array, including the ones that have already been matched, and fails for that reason. No reason to do that, preferred doesn't mean mandatory. Can you please check?

As to the other problematic test case, I guess the root cause of the issue is similar, please check again once the previous case has been dealt with.

@lochel lochel requested a review from ptaeuber May 13, 2019 14:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants