modelica / ModelicaStandardLibrary

Free (standard conforming) library to model mechanical (1D/3D), electrical (analog, digital, machines), magnetic, thermal, fluid, control systems and hierarchical state machines. Also numerical functions and functions for strings, files and streams are included.
https://doc.modelica.org
BSD 3-Clause "New" or "Revised" License
472 stars 169 forks source link

Discontinuous change in Modelica.Blocks.Tables.CombiTable2D #1465

Closed modelica-trac-importer closed 7 years ago

modelica-trac-importer commented 7 years ago

Modified by beutlich on 22 Dec 2016 09:58 UTC The output of Modelica.Blocks.Tables.CombiTable2D is discontinuous when crossing the 'boundary' of the interpolation table. Smoothness has to be set to ContinuousDerivative for this to be the case.

Code for an example:

model testModel ""

  Modelica.Blocks.Tables.CombiTable2D combiTable2D(table={{0,263.15,268.15,
        273.15,275.15,283.15,288.15,298.15},{308.15,6460,6970,5960,6010,6200,
        6310,6864},{318.15,7965,7850,7790,7780,7730,7690,7627},{328.15,0,0,9750,
        9700,9500,9380,9237},{333.15,0,0,0,8600,10300,10390,10169}}, smoothness
      =Modelica.Blocks.Types.Smoothness.ContinuousDerivative)
    annotation (Placement(transformation(extent={{-20,-22},{0,-2}})));
  Modelica.Blocks.Sources.Constant const(k=283)
    annotation (Placement(transformation(extent={{-78,-58},{-58,-38}})));
  Modelica.Blocks.Sources.Cosine cosine(
    freqHz=10,
    offset=333.15,
    amplitude=0.1)
    annotation (Placement(transformation(extent={{-80,-6},{-60,14}})));
equation 
  connect(const.y, combiTable2D.u2) annotation (Line(
      points={{-57,-48},{-40,-48},{-40,-18},{-22,-18}},
      color={0,0,127},
      smooth=Smooth.None));
  connect(cosine.y, combiTable2D.u1) annotation (Line(
      points={{-59,4},{-40,4},{-40,-6},{-22,-6}},
      color={0,0,127},
      smooth=Smooth.None));
  annotation (Diagram(coordinateSystem(preserveAspectRatio=false, extent={{-100,
            -100},{100,100}}), graphics));
end testModel;

I experience this issue on MSL 3.2.1 and Dymola 2014FD01


Reported by filip.jorissen on 20 Apr 2014 18:44 UTC The output of Modelica.Blocks.Tables.CombiTable2D is discontinuous when crossing the 'boundary' of the interpolation table. Smoothness has to be set to ContinuousDerivative for this to be the case.

Code for an example:

{{{model testModel ""

Modelica.Blocks.Tables.CombiTable2D combiTable2D(table={{0,263.15,268.15, 273.15,275.15,283.15,288.15,298.15},{308.15,6460,6970,5960,6010,6200, 6310,6864},{318.15,7965,7850,7790,7780,7730,7690,7627},{328.15,0,0,9750, 9700,9500,9380,9237},{333.15,0,0,0,8600,10300,10390,10169}}, smoothness =Modelica.Blocks.Types.Smoothness.ContinuousDerivative) annotation (Placement(transformation(extent={{-20,-22},{0,-2}}))); Modelica.Blocks.Sources.Constant const(k=283) annotation (Placement(transformation(extent={{-78,-58},{-58,-38}}))); Modelica.Blocks.Sources.Cosine cosine( freqHz=10, offset=333.15, amplitude=0.1) annotation (Placement(transformation(extent={{-80,-6},{-60,14}}))); equation connect(const.y, combiTable2D.u2) annotation (Line( points={{-57,-48},{-40,-48},{-40,-18},{-22,-18}}, color={0,0,127}, smooth=Smooth.None)); connect(cosine.y, combiTable2D.u1) annotation (Line( points={{-59,4},{-40,4},{-40,-6},{-22,-6}}, color={0,0,127}, smooth=Smooth.None)); annotation (Diagram(coordinateSystem(preserveAspectRatio=false, extent={{-100, -100},{100,100}}), graphics)); end testModel;}}}

I experience this issue on MSL 3.2.1 and Dymola 2014FD01


Migrated-From: https://trac.modelica.org/Modelica/ticket/1465

modelica-trac-importer commented 7 years ago

Modified by beutlich on 22 Apr 2014 08:30 UTC

modelica-trac-importer commented 7 years ago

Comment by beutlich on 28 Apr 2014 15:15 UTC Thank you for reporting this critical issue that was introduced in 7b1c1a5f5ae0507be71d6c2acd6cfb691377a9c4 for #1028.

I fixed this algorithmic bug by fae33b4ed66d563ee048fd7a2b1fe8d3ae264011 on source and binary level. For now I only commited to the trunk branch. Please test it thoroughly and in detail! If you confirm that fae33b4ed66d563ee048fd7a2b1fe8d3ae264011 fixes the problem I will also merge it to the maintenance branch of MSL 3.2.1.

modelica-trac-importer commented 7 years ago

Comment by beutlich on 5 May 2014 12:46 UTC @filip: Can you please comment on the fix fae33b4ed66d563ee048fd7a2b1fe8d3ae264011? Otherwise I will merge it to maint branch by 15th of May 2014. Thank you!

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 5 May 2014 13:12 UTC I believe that some additional description and consistency of the C-source would be helpful - both for the future and to make it easier to follow the logic of this change.

With consistency I mean that I first noticed that "return der_y" was added/removed, and then that der_y (and y) are sometimes returned at the end of functions, and sometimes as soon as possible.

modelica-trac-importer commented 7 years ago

Comment by beutlich on 5 May 2014 13:20 UTC Thanks for your feedback, Hans! I do really appreciate this!

Maybe you remember comment 8 of #1028 where I called it a mess. I will think about a necessary clean up.

modelica-trac-importer commented 7 years ago

Comment by anonymous on 5 May 2014 14:23 UTC Hello Beutlich,

thank you for the fix. Can I test the implementation somehow or would you like me to look at the code changes? I am afraid that verifying the code changes would too slow to be practical.

Filip

modelica-trac-importer commented 7 years ago

Comment by beutlich on 5 May 2014 14:42 UTC You might compile the ModelicaStandardTables lib yourself or use the updated binaries (that work for Dymola) provided by fae33b4ed66d563ee048fd7a2b1fe8d3ae264011.

modelica-trac-importer commented 7 years ago

Comment by filip.jorissen on 5 May 2014 14:51 UTC I'll see what I can do by the 15th!

Filip

modelica-trac-importer commented 7 years ago

Comment by beutlich on 5 May 2014 14:53 UTC Never mind the date. Now that I know that you are about to test I will wait for your feedback. Thanks!

modelica-trac-importer commented 7 years ago

Comment by beutlich on 8 May 2014 15:01 UTC In c39b6c91eb1712c1bdb9cea543dce5524a9b5aba I introduced new macros LINEAR, LINEAR_SLOPE and BILINEAR to indicate which kind of extrapolation is used. Moreover I merged the cases of interpolation and extrapolation for CombiTable1D and CombiTable2D in order to remove redundant code and restructure the branches.

Hans, can you please inspect again! Your comments are very welcome. Thanks.

@Filip, please use this update c39b6c91eb1712c1bdb9cea543dce5524a9b5aba for your test scenarios. Thanks.

modelica-trac-importer commented 7 years ago

Comment by filip.jorissen on 8 May 2014 16:08 UTC Hi Beutlich,

I verified the code by implementing the new binaries. The test model I proved runs ok and the model in which I found the bug also works. I did not check the code as I'm afraid it would not be a lot of use.

You have my blessing!

Filip

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 9 May 2014 15:15 UTC As indicated above I had to investigate a bit to understand the code, and added that as comments in 11ac46eaa797c937893a3a91cd80ccc7891dcaa9. The idea was to make it easier for others to understand the relation between the different branches, and also verify that there were no errors.

I understand that it might be possible to improve the syntax I used for the mathematical description; but at least it is a starting point.

I also tried to test it more and noticed that the derivatives were inconsistent with the values in some cases, I did this by replacing the const-signal with a ramp with offset 283 (and duration 1), i.e. replace the const-component above with:

  Modelica.Blocks.Sources.Ramp     const(
    offset=283,
    startTime=1,
    duration=1)

(and simulating to 2s). The derivatives are consistent inside the table, but the extrapolation differs between table and derivative. Using LINEAR_SEGMENTS also removed the inconsistency for this case.

I could see the cause of this when I got to ModelicaStandardTables_CombiTable2D_getDerValue for extrapolate1 == IN_TABLE and extrapolate2 == LEFT where the der_u1 terms are missing (and similarly for the other single-extrapolation cases). I did not update the C-code.

Note that I did not get to the formula for double-extrapolation cases.

modelica-trac-importer commented 7 years ago

Comment by beutlich on 9 May 2014 18:31 UTC Hans: Thanks for your investigation. I need to check and fix it again.

modelica-trac-importer commented 7 years ago

Comment by beutlich on 12 May 2014 14:35 UTC Hans: The derivatives should be fixed by 6809eb04e45b9f05fb9212a582cc77c6eb99550c. Can you please check again. Are your comments temporary or should they be kept? Thank you!

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 12 May 2014 15:09 UTC Replying to [comment:14 beutlich]:

@Hans: The derivatives should be fixed by 6809eb04e45b9f05fb9212a582cc77c6eb99550c. Can you please check again. Are your comments temporary or should they be kept? Thank you!

Thanks, I will verify.

The comments were intended to be kept, to help the next person reading the code.

modelica-trac-importer commented 7 years ago

Comment by beutlich on 12 May 2014 15:22 UTC Comments for getValue are still there, but getDerValue was completely refactored why I removed the comments. You may introduce them again if you like. Also instead of "u1-..." you could use "v1" etc.

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 14 May 2014 13:48 UTC It seems ok now when extrapolating one signal, but not when extrapolating both. I will attach test-cases where the problem occurs between 3 and 3.6s; the output does not look sine-like in that interval.

The analytic derivative is also different compared to numeric differentiation in that interval.

modelica-trac-importer commented 7 years ago

Comment by beutlich on 14 May 2014 14:12 UTC Thanks Hans, can reproduce it. Need to check :-(

modelica-trac-importer commented 7 years ago

Comment by beutlich on 15 May 2014 12:22 UTC Derivatives should be fixed by e001627bcf1d26873347f7bda27061d9e4ea18fd. There still was an error in the computation of the derivatives if one dimension was interpolated and the other one was extrapolated.

modelica-trac-importer commented 7 years ago

Modified by beutlich on 16 May 2014 21:05 UTC

modelica-trac-importer commented 7 years ago

Comment by beutlich on 19 May 2014 07:02 UTC Hans, can you please confirm that e001627bcf1d26873347f7bda27061d9e4ea18fd gives the expected outputs. Thank you!

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 19 May 2014 09:00 UTC Replying to [comment:21 beutlich]:

@Hans, can you please confirm that e001627bcf1d26873347f7bda27061d9e4ea18fd gives the expected outputs. Thank you!

I can confirm that the derivative is consistent with the function - even when extrapolating one or two the signals.

However, it looks as though there are jumps in the derivatives when switching from extrapolating one to extrapolating two of the signals which seems wrong.

modelica-trac-importer commented 7 years ago

Comment by beutlich on 20 May 2014 09:03 UTC Replying to [comment:22 hansolsson]:

However, it looks as though there are jumps in the derivatives when switching from extrapolating one to extrapolating two of the signals which seems wrong.

Hans, indeed, bivariate Akima is not differentiable if extrapolation switches from two signals to one signal. Please see my attached [attachment:T1465_Akima2D_Extrapolation.pdf document] where I - without loss of generality - considered the cases of

  1. left extrapolation of both u1 and u2 and (called yLL)
  2. interpolation of u1 and left extrapolation of u2 (called yTL) . You can see that the function value y (of course!) and the partial derivative dydu2 match at the border case but partial derivate dydu1 does not match in general.

What should we do here?

modelica-trac-importer commented 7 years ago

Comment by beutlich on 21 May 2014 10:59 UTC Hans, OK, we figured it out. Please wait for the final fix.

modelica-trac-importer commented 7 years ago

Comment by beutlich on 21 May 2014 13:46 UTC Hans, it should finally be fixed by 4e74e118a442b5ca2a975bf2a9f867107aa30c9d.

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 21 May 2014 14:56 UTC Replying to [comment:25 beutlich]:

@Hans, it should finally be fixed by 4e74e118a442b5ca2a975bf2a9f867107aa30c9d.

Ok, it looks good now.

modelica-trac-importer commented 7 years ago

Comment by beutlich on 21 May 2014 15:26 UTC Also resolved in MSL 3.2.1 maintenance branch by eeaca43baeacd3109a57fb2aa77cf1c6e5d65afc and 94859c37208368e5de6d196f359d2e8cd4d883a2.

Thank you, Hans, for your persistency in this issue.

modelica-trac-importer commented 7 years ago

Changelog modified by beutlich on 21 May 2014 15:26 UTC Fixed the linear and bilinear extrapolation of the bivariate Akima-splines of the Modelica Standard Tables

modelica-trac-importer commented 7 years ago

Modified by beutlich on 22 Dec 2016 09:58 UTC