fix: renaming of statistical procedures #44
Conversation
This commit unifies the sample generators for all mixture types (NMM, NMV, NV) by integrating canonical form support directly into the main generator function. Previously, each mixture type had separate classical and canonical generators. Now, the generator checks mixture_form inside the function and switches behavior accordingly. In canonical form, the beta parameter is not used. This simplifies the generator API and reduces code duplication. - Updated nm_generator.py, nmv_generator.py, nv_generator.py - Remembered `mixture_form` in AbstractMixture class - Updated tests and notebook to use the new unified API BREAKING CHANGE: canonical_generate() methods were removed; use generate() instead.
v1n0zavr1k
force-pushed
the
api/renaming-of-statistical-procedures
branch
from
930e698 to
9fa4aad
Compare
* added base StatisticalProcedure protocol * replaced NDArray[np.float64] with np.ndarray
andreev-sergej
left a comment
andreev-sergej
left a comment
There was a problem hiding this comment.
Overall, the changes are reasonable and correctly applied.
However, it looks like only the suggestions from #34 were followed and they are not complete. That results in some inconsistencies in naming. For example: SemiParametricGEstimationPostWidder was not renamed, while NMSemiParametricGEstimation became NMEstimationDensityInvMT.
Also, file names remain unchanged, which may lead to confusion, because they were inherited from old class names.
Merging this pull request would create a new task: renaming related test folders, files and classes.
|
|
||
|
|
||
| class NVSemiParametricGEstimationPostWidder: | ||
| class NMVEstimationDensityPW: |
There was a problem hiding this comment.
NV stands for Normal Variance, NMV -- for Normal Mean-Variance. I suggest renaming NVSemiParametricGEstimationPostWidder to NVEstimationDensityPW. I suppose you missed SemiParametricGEstimationPostWidder it should be renamed to NMVEstimationDensityPW. I know its messy. Thats why we refactor it : )
* Rename NVSemiParametricGEstimationPostWidder → NVEstimationDensityPW * Rename SemiParametricGEstimationPostWidder → NMVEstimationDensityPW * Rename folders in src/procedures * Rename tests/algorithms → tests/procedures lgorithms/semiparametric_sigma_estimation/__init__.py -> tests/procedures/nm_procedures/semiparametric_sigma_estimation/__init__.py
In the previous commit "NVSemiParametricGEstimationPostWidder" were mistakenly replaced by "NMVEstimationDensityPW"
- Updated `moment`, `pdf`, `cdf`, and `logpdf` methods to support both scalar and list inputs for efficient batch computation. - Extracted shared evaluation logic (e.g., `moment`, `pdf`, `cdf`, `logpdf`) into the `AbstractMixtures` base class to eliminate code duplication across subclasses. - Centralized input validation, RQMC integration, and distribution handling within the base class for better maintainability. - Fixed type annotations to align with abstract method signatures and resolve `mypy` type-checking issues. These changes improve code clarity, enable vectorized evaluations, and make future extensions easier to implement.
| NMVEstimationDensityPW, | ||
| ) | ||
| from src.procedures.semiparametric.nvm_semi_param_algorithms.g_estimation_given_mu import ( | ||
| from src.procedures.semiparametric.nvm_semiparametric.g_estimation_given_mu import ( |
There was a problem hiding this comment.
The directory is named nvm_semiparametric, but the class is called NMVEstimationDensityInvMTquad. The abbreviations NVM and NMV refer to the same thing but are written differently.
I suggest renaming the directory from nvm_semiparametric to nmv_semiparametric to make it consistent.
There was a problem hiding this comment.
Сlass is named NMVEstimationDensityInvMTquad, but the file containing it is still called g_estimation_given_mu —which comes from the class's old name. This can be a bit confusing.
I suggest renaming files to match their new class names. In this case i suggest to rename file to nmv_estimation_density_inv_mt_quad_rqmc_based.py, which is the class name converted to snake_case.
It seems like almost all files in src/procedurec need to be renamed.
feat: dynamic integrator selection
feat: unify mixture generators and support canonical forms
feat: add batch support and refactor mixture evaluation logic
plidan123
force-pushed
the
api/renaming-of-statistical-procedures
branch
from
e0b1aa9 to
0d635b3
Compare
…ures Api/renaming of statistical procedures
NDArray[np.float64]withnp.ndarrayas intermediate solution for unite style in all statistical procedures