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

Weibull has some misbehaving cases #310

Open
FreezyLemon opened this issue Nov 24, 2024 · 1 comment
Open

Weibull has some misbehaving cases #310

FreezyLemon opened this issue Nov 24, 2024 · 1 comment

Comments

@FreezyLemon
Copy link
Contributor

Might be part of #102.

  • When shape = +Inf and 0 < scale < 1: scale_pow_shape_inv (== scale.powf(-shape)) will become +Inf which causes all sorts of problems
    • cdf/sf/pdf/ln_pdf start returning NaN for valid params1
    • note: If scale >= 1, then cdf etc. generally behave well(?)
  • scale = +Inf will always make inverse_cdf(0) return NaN
    • exception: shape is also +Inf, then it returns 1...

I believe this behaviour is unpredictable and unwanted, but I'm not sure for every single case listed here. And I might've missed something.

Are parameters set to f64::INFINITY degenerate for this distribution? If they are not, then they need special handling from what I can tell.

Dirty testing code to find some things:

#[test]
    fn test_infinities() {
        let test_distrs = [
            create_ok(f64::INFINITY, 0.5),
            create_ok(f64::INFINITY, 1.0),
            create_ok(f64::INFINITY, 1.5),
            create_ok(0.5, f64::INFINITY),
            create_ok(1.0, f64::INFINITY),
            create_ok(1.5, f64::INFINITY),
            create_ok(f64::INFINITY, f64::INFINITY),
        ];

        for dist in test_distrs {
            println!("scale {}", dist.scale());
            println!("shape {}", dist.shape());
            println!("scale.powf(-shape) {}", dist.scale_pow_shape_inv);
            println!();
            println!("cdf(-1): {}", dist.cdf(-1.0));
            println!("cdf(0): {}", dist.cdf(0.0));
            println!("cdf(0.75): {}", dist.cdf(0.75));
            println!();
            println!("sf(-1): {}", dist.sf(-1.0));
            println!("sf(0): {}", dist.sf(0.0));
            println!("sf(0.75): {}", dist.sf(0.75));
            println!();
            println!("inverse_cdf(0): {}", dist.inverse_cdf(0.0));
            println!("inverse_cdf(0.75): {}", dist.inverse_cdf(0.75));

            println!();
            println!();
        }
    }

Footnotes

  1. x.powf(shape) will become x.powf(+Inf) which is zero for any x in (0, 1). Then it's 0 * scale_pow_shape_inv which is 0 * Inf which is NaN

@YeungOnion
Copy link
Contributor

Since shape=Inf converges to the delta distribution for all finite scale, I wouldn't call it degenerate, but much simpler to express than Weibull variable.


Perhaps I'm oversimplifying the issue, but I'm again wondering if we should disallow these not exactly degenerate, but still special case parameter values. (There are other cases where 0 values cause this sort of behavior).

I don't think most people would pass Inf intentionally. Personally, if I knew that I would just implement the explicit behavior.

I'm expecting that most of the time, we're going to be handling infinities that occurred from floating point operations instead of literal infinities. Our implementations are the behavior we expect for when a caller passes literal float infinity, so maybe this isn't what we want.

I don't think you got a full response on #303, but perhaps disallowing inf parameters would have culled more options at the start.

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

No branches or pull requests

2 participants