Skip to content

Conversation

@iraedeus
Copy link
Collaborator

No description provided.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds four new statistical distribution models to the mpest framework: Cauchy, Beta, Uniform, and Pareto Type I distributions. Each implementation includes parameter transformation utilities, sampling methods, probability density function calculations, and derivative computations for optimization purposes.

  • Full implementation of four new distribution models with both external and internal parameter representations
  • Comprehensive test suites for each distribution covering parameter conversion, sampling, PDF computation, and derivative calculations
  • Integration with the existing mpest model registry

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
mpest/models/uniform.py Implements Uniform distribution with L-moments parameter estimation
mpest/models/pareto.py Implements Pareto Type I distribution with log-transformed parameters
mpest/models/cauchy.py Implements Cauchy distribution with log-scale parameterization
mpest/models/beta.py Implements Beta distribution with log-transformed shape parameters
mpest/models/__init__.py Registers the new distribution models in the framework
tests/models/test_uniform.py Comprehensive test suite for Uniform distribution
tests/models/test_pareto.py Comprehensive test suite for Pareto distribution
tests/models/test_cauchy.py Comprehensive test suite for Cauchy distribution
tests/models/test_beta.py Comprehensive test suite for Beta distribution
Comments suppressed due to low confidence (4)

tests/models/test_pareto.py:45

  • [nitpick] The parameter name 'internal_params' is inconsistent with the function's purpose. Since this function tests converting FROM model parameters, the input should be named something like 'params' or 'model_params' to match the pattern used in other test files.
    def test_params_convert_from_model(self, internal_params):

tests/models/test_beta.py:113

  • [nitpick] The parameter name 'param' is inconsistent with other test methods which use 'params'. This should be 'params' for consistency.
    def test_pdf(self, x, param):

mpest/models/uniform.py:46

  • The method name 'lda' is ambiguous and unclear. Consider renaming to something more descriptive like 'ld_param_a' or 'derivative_wrt_a' to clearly indicate it's the derivative with respect to parameter 'a'.
    def lda(self, x: float, params: Params) -> float:

mpest/models/uniform.py:53

  • The method name 'ldb' is ambiguous and unclear. Consider renaming to something more descriptive like 'ld_param_b' or 'derivative_wrt_b' to clearly indicate it's the derivative with respect to parameter 'b'.
    def ldb(self, x: float, params: Params) -> float:

@iraedeus iraedeus merged commit 3073b3c into PySATL:main Jul 17, 2025
3 checks passed
@iraedeus iraedeus deleted the iraedeus/new-dists branch July 17, 2025 16:06
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.

1 participant