modelica-tools / csv-compare

Tool to compare curves from one csv files with curves from other csv files using an adjustable tolerance
https://www.modelica.org
BSD 3-Clause "New" or "Revised" License
25 stars 15 forks source link

Incorrect handling of trajectory endpoints #13

Open ghost opened 9 years ago

ghost commented 9 years ago

We have been seeing some issues with the false-negatives in the endpoints of some trajectory comparisons. It usually happens when the signal have a "high" derivative. For example: fig1 You can see that the bounds calculation doesn't account for the derivative of the trajectory at the end point. The two results are very close and should be accepted.

Example files can be found here: https://gist.github.com/jon-modelon/656e05349415bb2776a8

Command:

Compare.exe -o -m csvFileCompare -r . -t 0.001 --inline actual.csv expected.csv

Version:

Compare.exe --help CSV File Comparison Tool v2.0.0-rel Copyright ? 2015 ITI GmbH

tbeu commented 9 years ago
  1. Why was the tolerance decreased from 0.002 (= default) to 0.001?
  2. In general, what do you suggest for the minimal acceptable lower tube, i.e. the light blue horizontal segment at the end? We also need to consider the case with infinite slope resulting in acceptance of every possible result point near the end.
ghost commented 9 years ago
  1. We always run with 1e-3 for some historical reason, it's an arbitrary number. Even though changing to 2e-3 might fix this case the problem is still valid.
  2. My first intuition is that the tube should be extrapolated based on the derivative/slope of the tube bound at the endpoint(s?). If there is an infinite slope, then I'll guess that all points are valid (since the trajectory also has infinite slop in that point).
bastianbin commented 7 years ago

Hi ghost,

This is indeed a problem we have been thinking about. However, there're several reasons not to simply extrapolate the last slope when computing the polygonal chain. please check out thomas' response to this pull request.

We are, however, planning on adding a command line flag to enable pretty much what you have described. However, I cannot give you a time frame yet.