Skip to content

Conversation

@abpolym
Copy link
Member

@abpolym abpolym commented Nov 13, 2025

partition_point for Binomial, Categorical, NegativeBinomial, Weibull.

Resolves #49

Actually we should check conditioner in a general way before get_natural_manifold_base or partition_point are being called for the individual distributions - should avoid repetitive code.

…nt for Binomial, Categorical, NegativeBinomial, Weibull
@abpolym abpolym requested review from Nimrais and bvdmitri November 13, 2025 05:10
@abpolym abpolym linked an issue Nov 13, 2025 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Nov 13, 2025

Codecov Report

❌ Patch coverage is 83.33333% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.32%. Comparing base (ec2a281) to head (2d98826).

Files with missing lines Patch % Lines
src/natural_manifolds/binomial.jl 83.33% 2 Missing ⚠️
src/natural_manifolds/categorical.jl 83.33% 2 Missing ⚠️
src/natural_manifolds/negative_binomial.jl 83.33% 2 Missing ⚠️
src/natural_manifolds/weibull.jl 83.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #51      +/-   ##
==========================================
- Coverage   89.20%   87.32%   -1.88%     
==========================================
  Files          21       21              
  Lines         250      292      +42     
==========================================
+ Hits          223      255      +32     
- Misses         27       37      +10     

☔ 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.

@Nimrais
Copy link
Member

Nimrais commented Nov 14, 2025

I don't think that this is actually what issue is about to be honest. And these checks however could be usefull I do not see immidiatly why we need to add it, because we have these checks on the side of ExponentialFamily.jl.

Could you motivate your changes?

@bvdmitri
Copy link
Member

bvdmitri commented Nov 20, 2025

This is not a bad change and also doesn't hurt performance, but I would use Julia dispatch instead, for example this

function get_natural_manifold_base(::Type{Categorical}, ::Tuple{}, conditioner=nothing)
    if conditioner === nothing
        throw(
            ArgumentError(
                "get_natural_manifold_base(::Type{Categorical},..., conditioner): `conditioner` was left as `nothing`. Please provide a non-negative numeric conditioner (e.g. 0.0 or 1.0).",
            ),
        )
    end
    if !(conditioner isa Number)
        throw(
            ArgumentError(
                "get_natural_manifold_base(::Type{Categorical},..., conditioner): `conditioner` must be a Number, got $(typeof(conditioner)).",
            ),
        )
    end
    if conditioner < 1
        throw(
            ArgumentError(
                "get_natural_manifold_base(::Type{Categorical},..., conditioner): `conditioner` must be >= 1, got $(conditioner).",
            ),
        )
    end
    return ProductManifold(Euclidean(conditioner - 1), SinglePointManifold([0.0]))
end

can be rewritten as

check_manifold_conditioner(::Type{Categorical}, conditioner::Nothing) = throw(...)
check_manifold_conditioner(::Type{Categorical}, conditioner::Any) = throw(...)
check_manifold_conditioner(::Type{Categorical}, conditioner::Number) = conditioner > 1 ? nothing : throw(...)

and then

function get_natural_manifold_base(::Type{Categorical}, ::Tuple{}, conditioner=nothing)
        check_manifold_conditioner(Categorical, conditioner)
        return ProductManifold(Euclidean(conditioner - 1), SinglePointManifold([0.0]))
end

or this check can even be placed before the call of the get_natural_manifold_base in the top-level function

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.

Various user unfriendly errors for get_natural_manifold_base

4 participants