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

Set output float precision & relative epsilon for DD comparison #252

Merged
merged 4 commits into from
Dec 7, 2024

Conversation

siddharth-krishna
Copy link
Collaborator

@siddharth-krishna siddharth-krishna commented Dec 7, 2024

An attempted fix for #246, which was first noticed in #240. With some trial and error I set a precision of .10g (10 significant figures in exponential/scientific notation) for floating point numbers.

I also had to change the gdxdiff options to additionally use a tolerance in relative difference when comparing the output DD files to the ground truth, otherwise the higher precision output above increased the DD diff. We now use 1e-6 for both absolute Eps and relative RelEps difference tolerance. If I understand correctly how gdxdiff works, this feels like a reasonable tolerance:

AbsDiff := Abs(V1 - V2);
if  AbsDiff <= EpsAbsolute
then
  Result := true
else
  if EpsRelative > 0.0
  then
     Result := AbsDiff / (1.0 + DMin(Abs(V1), Abs(V2))) <= EpsRelative
  else
     Result := false;

Finally, we were reporting the number of lines in the output of gdxdiff, but that's only the number of sets/parameters that are different, and doesn't track improvements in number of rows different. I'm now passing the output of gdxdiff to gdxdump so that the "GDX Diff" column is more accurate and tracks number of rows in the diff.

@siddharth-krishna siddharth-krishna marked this pull request as ready for review December 7, 2024 11:56
@olejandro olejandro merged commit cbef3ab into main Dec 7, 2024
2 checks passed
@olejandro olejandro deleted the sid/diff-precision branch December 7, 2024 13:05
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 this pull request may close these issues.

2 participants