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

fixing issue #1507 by using FSM #1513

Closed
wants to merge 1 commit into from
Closed

fixing issue #1507 by using FSM #1513

wants to merge 1 commit into from

Conversation

hbina
Copy link
Contributor

@hbina hbina commented Jan 25, 2019

Description

WHAT
This PR attempts to fix Issue #1507 by introducing a finite state machine (FSM) to determine the type of approximation that the user wants to perform. The default is using both and then OR'ing them.

WHY
As per Issue #1507 the inequality check will fail even though it is obviously correct. More simply, the equality is supposed to be wrong but Catch2 is reporting them as correct, because we performed an OR of epsilon_check and margin_check.

The problem seems to occur when the margin that is set by the user is smaller than the default epsilon, therefore, while margin_check will fail, epsilon_check will not.

GitHub Issues

Closes #1507

…explicitly trying to use epsilon or margin
@codecov
Copy link

codecov bot commented Jan 26, 2019

Codecov Report

Merging #1513 into master will increase coverage by 0.08%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1513      +/-   ##
==========================================
+ Coverage   80.53%   80.61%   +0.08%     
==========================================
  Files         121      121              
  Lines        3436     3451      +15     
==========================================
+ Hits         2767     2782      +15     
  Misses        669      669

@hbina
Copy link
Contributor Author

hbina commented Jan 28, 2019

The original Issue #1507 was marked as Resolved since this behaviour is intended. Closing the PR.

@hbina hbina closed this Jan 28, 2019
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.

Approx is incorrect for small margins
1 participant