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

Add benchmarking of regStep vs spliceFunction #300

Closed
marcusfuchs opened this issue Jul 20, 2015 · 15 comments · Fixed by #434
Closed

Add benchmarking of regStep vs spliceFunction #300

marcusfuchs opened this issue Jul 20, 2015 · 15 comments · Fixed by #434
Assignees

Comments

@marcusfuchs
Copy link
Contributor

For FMU-Paper.

@marcusfuchs
Copy link
Contributor Author

I added simple tests to the Benchmarks package. I ran each test 10 times and compared the computation times for each call of the block "Auxiliary2 code", which I think includes the function calls.

The results showed quite some variations, with the regStep function seeming to be a bit faster than the splice function on average:

regStep() spliceFunction()
57.08 61.98
55.18 59.68
50.68 63.47
52.38 59.38
52.88 58.68
55.58 56.58
59.28 64.27
53.68 55.68
58.48 55.18
49.48 55.58

Results are in us per call of block.

@mwetter
Copy link
Contributor

mwetter commented Jul 20, 2015

Because regStep is on average about 8% faster, I think we should use regStep in Utilities.Math.

@Mathadon
Copy link
Member

Did you verify if the function is inlined? This could create a speed up. You can write An algorithm that loops over the function. That makes it easier to compare performance.

Op 21-jul.-2015 om 01:30 heeft Michael Wetter <notifications@github.commailto:notifications@github.com> het volgende geschreven:

Because regStep is on average about 8% faster, I think we should use regStep in Utilities.Math.


Reply to this email directly or view it on GitHubhttps://github.com//issues/300#issuecomment-123090532.

@marcusfuchs
Copy link
Contributor Author

That is a good point, I had originally not taken that into account.

Now I duplicated both regStep() and spliceFunction() and added inline=true to both functions. With both regStep() and regStepInline(), the code seemed to be inlined in the dsmodel.mof, while for both spliceFunction() as well as for spliceFunctionInline(), the code was not inlined, despite the annotation inline=true for spliceFunctionInline(). So that may also be part of the speed difference.

@mwetter
Copy link
Contributor

mwetter commented Jul 27, 2015

@marcusfuchs : Did you use inline or Inline? It should be capitalized, but I could not check your implementation as it is not on github.

@marcusfuchs
Copy link
Contributor Author

You're right, I changed it to Inline, but I was not able to see any changes in the dsmodel file. Yet, as I had not used Inline before, there may still be something wrong with my implementation. I pushed the examples to the branch.

@mwetter
Copy link
Contributor

mwetter commented Jul 28, 2015

@marcusfuchs The implementation seems correct. Note that "Inline" is only a suggestion for the translator. Maybe because spliceFunction is larger, Dymola decides not to inline the code.

Next, I suggest to use regStep where we now use spliceFunction, and I can run a few tests on the larger models of the Buildings library, preferably after #301 is corrected

@Mathadon
Copy link
Member

Mathadon commented Aug 2, 2015

From my experience with inlining Dymola will only inline the function if the function body contains only one equation (including parameters, i.e. no parameters should exist). This is the case for Annex60.Experimental.Benchmarks.Utilities.FunctionsInlined.regStepInline but not for Annex60.Experimental.Benchmarks.Utilities.FunctionsInlined.spliceFunctionInline. With following implementation the function is inlined:

function spliceFunctionInline "Spline interpolation of two functions"
  extends Modelica.Icons.Function;
  input Real pos "Returned value for x-deltax >= 0";
  input Real neg "Returned value for x+deltax <= 0";
  input Real x "Function argument";
  input Real deltax=1 "Region around x with spline interpolation";
  output Real out;
algorithm 
  out := (pos-neg)*(if x/deltax <= -0.999999999 then 0 elseif x/deltax >= 0.999999999 then 1 else (Modelica.Math.tanh(Modelica.Math.tan(x/deltax*Modelica.Math.asin(1))) + 1)/2) + neg;

  annotation (Inline = true,
              derivative=Modelica.Media.Air.MoistAir.Utilities.spliceFunction_der);
end spliceFunctionInline;

@mwetter
Copy link
Contributor

mwetter commented Dec 21, 2015

In the last comparison, regStep was faster but this may have been due to the inlining. The next steps are

  • implement the inlined function in a branch (if not done already)
  • @marcusfuchs to repeat the benchmarks with both functions being inlined to compare the computing time. It seems that the benchmark code from Marcus is not yet on github
  • compare the cpu time and merge the faster one to the master branch

marcusfuchs pushed a commit that referenced this issue Dec 29, 2015
@marcusfuchs
Copy link
Contributor Author

I added a Python script that compares regStep, regStepInline, splice, and spliceInline with 10 runs each in random order to the bin directory. For the spliceInline I changed the benchmark implementation to the one suggested by @Mathadon, and to me it seems like this one indeed gets inlined, while the former implementation was not inlined. The test I ran shows that regStepInline was fastest:

regstepvssplice

@Mathadon
Copy link
Member

Mathadon commented Jan 4, 2016

I think we have a clear winner :) Also I think that the actual computation time difference may be large than what is indicated by these box plots since there is a lot of overhead for compiling, initialisation, etc?

@marcusfuchs
Copy link
Contributor Author

I ran one test for total CPU time and, again, regStepInline was fastest:

regstepvssplice

@mwetter
Copy link
Contributor

mwetter commented Jan 5, 2016

  • Yes, let's go with the inlined version of regStep and replace all uses of the spliceFunction except of course in Annex60.Utilities.Math.Splice and in Annex60.Utilities.Math.Functions.splice.

Maybe these two should be moved to Obsolete and instead implementations of regStep added as I don't see a situation where a user would want to use spliceFunction rather than regStep

  • We should also check whether smoothMin will be inlined as it calls smoothMax which implements the regularization. We want to implement regStep in both of these directly rather than calling another function if this avoids getting inlined.

marcusfuchs pushed a commit that referenced this issue Feb 18, 2016
For #300. As regStepInline was found to be the implementation with best
performance, this is moved from Experimental into the Utilities package.
@mwetter
Copy link
Contributor

mwetter commented Mar 15, 2016

@marcusfuchs : I started implementing your changes. I merged the relevant 3 files (not the benchmarks) to a new branch called issue300_regStep_integration and renamed regStepInline to regStep. I am now testing the new code.

@mwetter
Copy link
Contributor

mwetter commented Mar 16, 2016

In some cases, nonlinear equations are replaced by linear equations after using regStep.

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 a pull request may close this issue.

3 participants