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

Use Xi as input for source models & removing BaseProperties #882

Closed
Mathadon opened this issue Jan 28, 2018 · 4 comments
Closed

Use Xi as input for source models & removing BaseProperties #882

Mathadon opened this issue Jan 28, 2018 · 4 comments
Assignees

Comments

@Mathadon
Copy link
Member

Currently, the models in IBPSA.Fluid.Sources allow the use of an input for X, while in many cases X is not directly available. E.g. a FluidPort only contains the independent fractions Xi. I would like to add the option to use Xi as an input instead of X to avoid having to do obsolete computations to compute X from Xi. This process is error-prone and annoying since you're often working with multidimensional vectors. I can't see any downside if we add this option in an advanced tab to avoid confusion with users about the difference between using X and Xi as an input.

A related issue: these models instantiate Medium.BaseProperties. I'm not sure why this is the case, possibly to the check consistency of X, temperatures that are out of the bounds, etc? In any case, I'd like to add the option to remove this instantiation since it introduces additional equations and/or translation time to the problem.

Any remarks before I implement this?

@Mathadon Mathadon self-assigned this Jan 28, 2018
@mwetter
Copy link
Contributor

mwetter commented Jan 29, 2018

Both are worthwhile, but maybe we can move X to the advanced tab rather than Xi. Xi is more useful in my view.

Can you check whether removing Medium.BaseProperties indeed gives faster translation and simulation. I would think the overhead is small, and it allows to catch unreasonable user input.

@Mathadon
Copy link
Member Author

Mathadon commented Feb 2, 2018

Using this example

model Boundary
  parameter Integer n = 200;
  Boundary_pT[n] bou(
    redeclare package Medium = Medium,
    each use_p_in=true,
    each use_T_in=true);
  Modelica.Blocks.Sources.Cosine cosine(freqHz=1, offset=1e5);
  Modelica.Blocks.Sources.Cosine cosine1(offset=290);
  replaceable package Medium = IBPSA.Media.Air;
equation 
  for i in 1:n loop
    connect(bou[i].p_in, cosine.y);
    connect(cosine1.y, bou[i].T_in);          
  end for;
end Boundary;

I get following translation/simulation times (seconds) for 1e5 steps.

With BaseProperties

n=200 - 15/0.555
n=400 - 21/1.19
n=600 - 32/1.61

Without

n=200 - 5/0.0344
n=400 - 5.5/0.0328
n=600 - 7/0.0371

The low simulation times in the second case may however be due to alias variable eliminations..

I'll make BaseProperties conditional with default enabled?

@Mathadon
Copy link
Member Author

Mathadon commented Feb 2, 2018

I verified the analysis with

model Boundary
  parameter Integer n = 200;
  IBPSA.Fluid.Sources.Boundary_pT[n] bou(
    redeclare package Medium = Medium,
    each use_p_in=true,
    each use_T_in=true);
  Modelica.Blocks.Sources.Cosine[n] cosine(freqHz=1:n, offset=1e5);
  Modelica.Blocks.Sources.Cosine[n] cosine1(freqHz=1:n, offset=290);
  replaceable package Medium = IBPSA.Media.Air;
equation 

    connect(bou.p_in, cosine.y);
    connect(cosine1.y, bou.T_in);

end Boundary;

which results in comparable results. The clue could be that the asserts are function calls that cause a lot of overhead.

Mathadon pushed a commit that referenced this issue Feb 2, 2018
added option for Xi input in sources
see #882
Mathadon pushed a commit that referenced this issue Feb 3, 2018
@Mathadon
Copy link
Member Author

Mathadon commented Feb 3, 2018

Translation/simulation times for the example above (n=200), commit 1aa9676:

Old implementation with input verification: 20s / 3.7 s
New implementation with input verification: 18s / 3.6 s
New implementation without input verification: 8.5s / 0.0556 s

Mathadon pushed a commit that referenced this issue Feb 3, 2018
Mathadon pushed a commit that referenced this issue Feb 13, 2018
Mathadon pushed a commit that referenced this issue Feb 16, 2018
to update connections of IBPSA.Fluid.Sources.MassFlowSource_h and IBPSA.Fluid.Sources.MassFlowSource_T
@mwetter mwetter closed this as completed Feb 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants