BugFix: Scale Microgrid Error

[reid-g]

This pull request fixes issue #16 .

Changed the error estimation to norm based instead of element wise. All test cases pass now. Solution vectors from the same system solved as a DAE are provided (from MATLAB).

Numerical differences in elements of the vector seems to be bottoming out any improvements on error. Step size drops to be quite small initially as well (h_0 < reltol). However, as discussed this does not seem to be an issue.

[nkoukpaizan]

I am a little worried that moving to the two norm is not fixing the underlying issue and may give a false-negative. If we expect the vectors to be close element-wise, then that should be the right thing to check for the test.

[pelesh]

The tolerances in the example need to be better documented. Use Doxygen @todo keyword to document issues we have not clarified yet.

[pelesh]

There has to be comment justifying this tolerance. How was it estimated?

[pelesh]

By the way, all tolerances should be defined in SolutionKeys.hpp file because they depend on how these values in the solution key are generated. I would implement it like:

Suggested change

	real_type error_tol = 8e-5;
	real_type error_tol = MICROGRID_TOL;

and define MICROGRID_TOL in the SolutionKeys.hpp file. There should be an explanation why you picked 8e-5 and not 7e-5, for example. The explanation should be commented in the code using Doxygen format.

[reid-g]

This was roughly the best error obtained.
The N=8 case hovers around 1e-5 to 3e-5 when reltol=1e-7; abstol=1e-7. This the point where the error on the N=8 system "bottoms out". Higher and lower tolerances increase error. error_tol was chosen based of this.

[pelesh]

I think how you came up to the tolerance used for testing needs to be documented in the code. The argument for selecting that specific tolerance is unconvincing. Eventually we will want to improve this test and it would be good to have documented what has been done to this point.

Also, I would put numerical value for the tolerance and the comments in the SolutionKeys.hpp file.

[reid-g]

Added documentation on why it was chosen. Results are commented in showing why as well.

[pelesh]

Which one of the two is true?

[reid-g]

Both give correct solutions. The DAE formulation has slightly better 2-norm error (due to different numerical algorithms). I left them in for comparison since the model has trivial constraints. Should one be removed?

[pelesh]

There is only one set of data with reference results. Comments in the code should describe only the data that is included.

[reid-g]

??? There are two set of keys. ODE keys are denoted answer_key_N* and DAE keys are denoted answer_key_N*_DAE. Both reference data sets are in the solution keys file.

[pelesh]

I am a little worried that moving to the two norm is not fixing the underlying issue and may give a false-negative. If we expect the vectors to be close element-wise, then that should be the right thing to check for the test.

SUNDIALS is using weighted 2-norm to compare with relative tolerance. Ideally, we should use the same. For this PR, it should be at least documented where these tolerances come from, why particular numbers are used and why we believe this is a reasonable verification of results.

[reid-g]

I am a little worried that moving to the two norm is not fixing the underlying issue and may give a false-negative. If we expect the vectors to be close element-wise, then that should be the right thing to check for the test.

SUNDIALS is using weighted 2-norm to compare with relative tolerance. Ideally, we should use the same. For this PR, it should be at least documented where these tolerances come from, why particular numbers are used and why we believe this is a reasonable verification of results.

The weighted 2-norm used is an estimate of temporal error. Since we have a reference solution to compare then the raw temporal accuracy measure I think should be used (standard 2norm relative error).

It was my mistake to originally have the element-wise relative error setup. The methods accuracy is bounded from analysis by the 2norm. Not the element-wise.