Wrap approx

[YeungOnion]

Implements wrapper for approx crate so that we can more easily set defaults across the crate. Should lead to better understanding of "overall" precision, there's also a fairly reasonable way to redefine default precision at the module level, I included this as a sample in function::beta.

I'd be open to any feedback to make it a little more ergonomic, the usage is added in f3fbf98

[codecov]

Codecov Report

Attention: Patch coverage is 99.79101% with 2 lines in your changes missing coverage. Please review.

Project coverage is 95.07%. Comparing base (a2b17af) to head (8f97ade).

Files with missing lines	Patch %	Lines
src/distribution/geometric.rs	92.30%	1 Missing ⚠️
src/distribution/levy.rs	90.90%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #325      +/-   ##
==========================================
- Coverage   95.27%   95.07%   -0.21%     
==========================================
  Files          60       60              
  Lines       14291    13518     -773     
==========================================
- Hits        13616    12852     -764     
+ Misses        675      666       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[YeungOnion]

@FreezyLemon would you be willing to provide some feedback on the following?

• ergonomics to use within lib and tests
• ergonomics in setting defaults at other scopes. ¹
• Prose in the README.md

Footnotes

1. I may want to document somewhere that we won't approve requests for lesser precision than that in prec as the intent of having it is to help someone appraise what the library offers. ↩

[FreezyLemon]

Of course. It might be a few days until I can take a look though.

[FreezyLemon]

Hm.. are these new macros in prec.rs intended to be part of the public crate API, or are they just there as helpers for testing? It's not obvious to me at first glance.

Regarding the ergonomics, I think the wrapper macros are probably as useful as the original approx macros (they seem to just emulate their API if I read it correctly).

But why do we need a macro to set module-level defaults? Wouldn't it be way easier (to write and read) to just create a one-line wrapper function?

// local wrapper to change default epsilon
fn ulps_eq(a: f64, b: f64) -> bool {
    crate::prec::ulps_eq!(a, b, epsilon = 1e-14)
}

Macros can be useful, but I suggest preferring normal functions over macros if possible. It may also make sense to introduce functions for some things that are macros at the moment. E.g. ulps_eq is almost always called with the default epsilon/max_ulps values.

[FreezyLemon]

Note: The commit (8ad4eed) introducing this/moving this from lib.rs should be marked as breaking. It is currently part of the public API and this PR makes it effectively pub(crate).

[FreezyLemon]

After seeing that AI was used to create some of this paragraph: Have you checked that these sources exist and are useful for the stated purpose?

[YeungOnion]

I think these sources are valuable, and not all were generated, but I'd be open to changing to those selected by the numerics community, perhaps from Julialang.

It was encouraging to see that they also link to Bruce Dawson's blog, which I chose to link to since I've found it in many places.

[YeungOnion]

Let me know what you think. The intent of prec is to offer shared defaults for contributors while also providing a high level overview of precision for those considering statrs as a dependency.

[FreezyLemon]

Sorry, it took me a while to get back to this. This is a bit daunting to review because of the number of changes all across the crate (which to me is a sign that this consolidation definitely needs to happen!). There is a real possibility I missed something, but these changes are important so I'll just go ahead with it.

I'm still not thrilled about the amount of macros in prec.rs, but it's not a big problem.

I like this change a lot, and it will probably be a nice improvement in the long term.

[FreezyLemon]

This has not been changed in this PR, but I just realized that this is incorrect. would you mind including a fix for this here?

[YeungOnion]

Alright. Since the API is to use the bottom level macros we can always reimplement those explicitly when someone inevitably finds a better way to express module level defaults.

Should be able to get through the conflict today and merge.

[YeungOnion]

github isn't able to check auto merge but has no issues per githubstatus.com an issue¹. I did git merge --ff-only on my machine and will push to master, the CI passed and then the mergability froze, but the codecov reports didn't upload - so I'm doubtful that the CI actually ran.

Below I'm running what of the CI I could do myself,

~/p/statrs master• ↑11 10.7s ❱ cargo test --doc
# output removed for brevity
test result: ok. 189 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 10.22s

~/p/statrs master• ↑11 2.5s ❱ cargo nextest run
# output removed for brevity
     Summary [   2.048s] 703 tests run: 703 passed, 4 skipped

info: --no-dev-deps modifies real `Cargo.toml` while cargo-hack is running and restores it when finished
info: running `cargo check --no-default-features` on statrs (1/7)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.02s

info: running `cargo check --no-default-features --features rand,std` on statrs (2/7)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.03s

info: running `cargo check --no-default-features --features default` on statrs (3/7)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s

info: running `cargo check --no-default-features --features nalgebra` on statrs (4/7)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.03s

info: running `cargo check --no-default-features --features rand` on statrs (5/7)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.02s

info: running `cargo check --no-default-features --features nalgebra,rand` on statrs (6/7)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.03s

info: running `cargo check --no-default-features --features std` on statrs (7/7)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.02s

I'll wait to see if it's ephemeral.

Footnotes

1. I found the issue before it appeared up there, ↩

[YeungOnion]

Will merge this since the wrapper for approx seems correct and merged. Also will follow with a fix for testing that only one relation between cdf and pdf is numerically close.