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.

Sign up for GitHub

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

Jump to bottom

Implement kwargs for isapprox #20

Closed
tpgillam opened this issue Dec 18, 2023 · 1 comment · Fixed by #22
Closed

Implement kwargs for isapprox #20

tpgillam opened this issue Dec 18, 2023 · 1 comment · Fixed by #22
Assignees
@tpgillam
Labels
enhancement New feature or request

Comments

@tpgillam
Copy link
Collaborator

Currently we don't accept rtol or atol keyword arguments. For compatibility with the API one expects from the standard implementations of Base.isapprox, we should do so.
@tpgillam tpgillam added the enhancement New feature or request label Dec 18, 2023
@tpgillam
Copy link
Collaborator Author

This actually causes problems due to the fact that the GA types currently are subtypes of Number... it means that when rtol is specified, we actually end up trying to call the version of isapprox in Base that is implemented for (::Number, ::Number), but also supports kwargs.

This seems counterintuitive, but it's a consequence of how Julia implements kwargs dispatch right now. See here: JuliaLang/julia#9498

@tpgillam tpgillam self-assigned this Dec 19, 2023
@tpgillam tpgillam mentioned this issue Dec 20, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tpgillam tpgillam

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add kwargs to isapprox MonumoLtd/SimpleGA.jl
1 participant
@tpgillam