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

Initial checkin for a module for tolerant comparison of reals #353

Closed
wants to merge 3 commits into from

Conversation

arjenmarkus
Copy link
Member

The module is based on the idea to allow for a small margin when comparing reals. Such a margin can solve the problem that the finite precision of floating-point calculations make the results inaccurate. Two essentially equal real numbers may therefore appear to be different. This is in response to issue #327

The module is based on the idea to allow for a small margin when comparing reals. Such a margin can solve the problem that the finite precision of floating-point calculations make the results inaccurate. Two essentially equal real numbers may therefore appear to be different
This backup file (from an edit action) was not supposed to go into the repository
@arjenmarkus
Copy link
Member Author

arjenmarkus commented Mar 20, 2021 via email

@milancurcic
Copy link
Member

Thanks @arjenmarkus, I think these will be useful.

Is "tolerant" a commonly and uniquely used term? I wonder if most people would understand what it is just based on that word.

What do you think about stdlib_comparisons? But then it misses the point of the comparisons being tolerant. And stdlib_tolerant_comparisons is unwieldy.

I guess my main gripe with the word "tolerant" is that it's an adjective and not a noun, i.e. it doesn't say what is in the module, but what something is like in the module.

@arjenmarkus
Copy link
Member Author

arjenmarkus commented Mar 23, 2021 via email

@milancurcic
Copy link
Member

Currently stdlib_comparison or stdlib_comparisons is my favorite. We can request feedback on the call tomorrow.

@arjenmarkus
Copy link
Member Author

arjenmarkus commented Mar 24, 2021 via email

@awvwgk awvwgk added reviewers needed This patch requires extra eyes API Discussion on a specific API for a proposal (exploratory) labels Apr 17, 2021
Copy link
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank ypou @arjenmarkus for this PR. Here are some comments.
I agree with @milancurcic: stdlib_comparison or stdlib_comparisons would be better thatn stdlib_tolerant. However, they don't cover tfloor,,... What do you think about stdlib_operators?

General comments:

  • Some hyperlinks mst still be added in the code, but this could be done at the end of the PR
  • fypp could be used to generated the code for the different kinds of real.


Provide tolerant versions of the `floor`, `ceil` and `round` functions that take a small interval into account.
While the actual interval can be set, the advised default is three times epsilon(). Note that the implementation
is actually much more involved than would seem necessary. It is the result of extensive research.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
is actually much more involved than would seem necessary. It is the result of extensive research.
is actually much more involved than would seem necessary. It is the result of extensive research.

#### Description

Provide tolerant versions of the `floor`, `ceil` and `round` functions that take a small interval into account.
While the actual interval can be set, the advised default is three times epsilon(). Note that the implementation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
While the actual interval can be set, the advised default is three times epsilon(). Note that the implementation
While the actual interval can be set, the advised default is three times `epsilon()`. Note that the implementation


`x`: The real number that is to be truncated or rounded

`ct`: Tolearance for comparison (optional, defaults to 3*epsilon)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`ct`: Tolearance for comparison (optional, defaults to 3*epsilon)
`ct`: Tolerance for comparison. It is an `intent(in)` and `optional` argument. The default is 3*`epsilon().


#### Arguments

`x`: The real number that is to be truncated or rounded
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`x`: The real number that is to be truncated or rounded
`x`: The real number that is to be truncated or rounded. It is an `intent(in)` argument.

Comment on lines +27 to +28
integer, parameter, private :: sp = kind(1.0)
integer, parameter, private :: dp = kind(1.0d0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to use the ones defined in stdlib_kinds?


#### Arguments

`x`, `y`: Two reals (of the same kind) to be compared
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are they scalar? Or are arrays also possible (and then the function should be elemental?

Comment on lines +85 to +87
logical function teq_sp( x, y ) result(cmp)
real(kind=sp), intent(in) :: x, y

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the functions for the different kinds could be generated with fypp.


implicit none

integer, parameter :: dp = kind(1.0d0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe using the ones defined by stdlib_kinds?

Comment on lines +12 to +16
real(kind=dp) :: x, y, z
real(kind=dp) :: yfloor, yceil
integer :: i

real(kind=dp) :: eps3 = 3.0_dp * epsilon(eps3)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fypp could be used to generate the same tests for the other kinds.

@jvdp1 jvdp1 added the waiting for OP This patch requires action from the OP label Jul 25, 2021
@awvwgk awvwgk added topic: mathematics linear algebra, sparse matrices, special functions, FFT, random numbers, statistics, ... and removed reviewers needed This patch requires extra eyes labels Sep 18, 2021
@awvwgk
Copy link
Member

awvwgk commented Dec 19, 2021

With #488 this functionality was implemented, therefore I'm going to close this PR.

Feel free to reopen this PR if you think #488 doesn't address all changes included in this patch.

@awvwgk awvwgk closed this Dec 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Discussion on a specific API for a proposal (exploratory) topic: mathematics linear algebra, sparse matrices, special functions, FFT, random numbers, statistics, ... waiting for OP This patch requires action from the OP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants