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

First steps toward quadrature #146

Merged
merged 12 commits into from
Feb 18, 2020
Merged

Conversation

nshaffer
Copy link
Contributor

@nshaffer nshaffer commented Feb 6, 2020

Here is a starting point for the proposed quadrature module (#112)

  • trapz, trapz_weights

    • Simple API, implementation, and tests for 1-d real arrays only
    • TODO: complex arrays (straightforward for 1-d)
    • TODO: multi-dimensional arrays (still some API questions to resolve)
  • simps, simps_weights

    • API for 1-d real arrays
    • TODO: Implementation & tests
    • TODO: complex arrays
    • TODO: multi-dimensional arrays

I did not get as far as I hoped in the past week, put I want to get some baseline code up for others to work off of.

Some points I'm hoping for some specific feedback on:

  • Edge cases. Do we like the proposed handling of small arrays?
  • The even argument of simps and simps_weights. Does this seem like a reasonable thing to do? Does the API make sense?

@nshaffer nshaffer marked this pull request as ready for review February 6, 2020 13:52
Copy link
Member

@certik certik left a comment

Choose a reason for hiding this comment

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

I think this looks good.

One comment is that I think floating point numbers should not be compared using just call assert(val == ans), but rather as call assert(abs(val - ans) < eps), otherwise we are just asking for trouble. I know we discussed this and @nncarlson suggested in some cases we might compare them directly.

Also note that gfortran actually warns in all such cases:

test_optval.f90:66:16:

     call assert(foo_qp() == 2.0_qp)
                1
Warning: Equality comparison for REAL(16) at (1) [-Wcompare-reals]

And we should strive to not get any warnings when compiling. Even if sometimes we might disagree with the warning, the general idea is that if the code is warning free, then there will not be problems with floating point comparisons.

@nshaffer
Copy link
Contributor Author

nshaffer commented Feb 7, 2020

@certik Hm, I'm of two minds about that. Sometimes you're testing that a function return a specific value, and it is incorrect behavior if it differs even by one bit. That's the case in optval and for my edge-case tests for trapz. But I do like the ideal of warning-free compilation. I've gone ahead and changed all the real equality tests here to tolerance checks against epsilon.

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.

It looks good to me. I just have a small comment.
It can be merged IMO.

@milancurcic milancurcic merged commit f063146 into fortran-lang:master Feb 18, 2020
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.

4 participants