[bugfix][checkDerivation] The test should fail if either tolerance is violated.

[dr-robertk]

Currently tests fail only if both tolerances are exceeded. I also think that the absolute tolerance does not make much sense anyway.

[pgdr]

This is not an objection to this PR or anything related to the C++ ECL test framework we have in this repo.

But does it make sense to enforce that the absolute difference between certain parameters is on the order of 0.01 over two different runs of flow? Aren't these values often on the order of several thousands, not to say millions?

E.g., if BPR after 5 years of running a simulation is 7465.67 instead of 7465.69, is this really something that should halt development until opm-data has been rerun? Wouldn't it be more interesting to compare with Eclipse output and try to stay within, say, 10% from that?

[GitPaean]

I think for small values, the absolute difference matters.

For big values, the relative difference matters.

[andlaus]

I think it depends on the quantity which is looked at: if the valid range is unconstrained -- like e.g. pressure -- the relative deviation is the relevant one, while for quantities with a limited range -- e.g., saturations, mass fractions, etc. -- the absolute difference is the one that matters. In practice, this probably boils down to @GitPaean's statement.

[GitPaean]

In my opinion, && probably works okay in term of filtering out the irrelevant deviation. || will possibly impose too strict standard.

[joakim-hove]

I seem to remember that @akva2 had some opinion on this; personally I do not have any strong opinion and will leave it to @akva2 - unless told otherwise.

[alfbr]

A lot of testing and thought was put into the tolerances. Still, I guess having uniform rules for all vectors were bound to show limitations sooner or later. Now may very well prove to be later. Please allow us some time to investigate the best way forward.

[pgdr]

I agree with @GitPaean that this will be too strict. This is also evident at Travis'

For keyword WWPR:PRODU13
(days, reference value) and (days, check value) = (170, 2.7711e-16) and (170, -0)
The absolute deviation is 2.7711e-16. The tolerance limit is 0.2
The relative deviation is 1. The tolerance limit is 4e-05

That's 2.8e-16 vs 0 difference after 170 days.

[akva2]

this works as intended. in general, the relative check is the 'active' one, picking up changes, and the absolute tend to fail repeatedly. however, for values close to zero, a relative check is not very useful. there the absolute check is the one which makes sense. ie what @GitPaean said.

[akva2]

@pgdr the && is in the failure condition so it behaves exactly like you want it to apart from the reference you want to use.

changing these tests to compare to eclipse reference makes them into something completely different. their purpose is not acceptance checking, they are for regression testing. we want to know whenever this output changes (above tolerances). it's a feature, not a burden, even though it obviously may feel as the latter at times. but all good things come at a cost.

there are acceptance/integration/call-it-what-you-want tests for polymer, norne, spe accessible through the run triggers so we have those as well. but as i said, these are a different class of tests with a different purpose.

[dr-robertk]

This discussion should probably continued after the release.

[joakim-hove]

Should this discussion be continued - or closed?

[joakim-hove]

Well - this seems stale; repoen if relevant.