Skip to content

Conversation

@LeonidElkin
Copy link
Collaborator

@LeonidElkin LeonidElkin commented Nov 7, 2025

Done:

  • add discrete distribution support
  • refactore old registry graph
  • add more tests to new graph
  • delete surprisingly useless GenericCharacteristic

Need to be done:

  • Lots of tests and thoroughly checked current
  • Lots of flexible fitters
  • Log likelyhood moved to indefinitvie
  • example update

@LeonidElkin LeonidElkin requested a review from Desiment November 7, 2025 12:34
@Desiment
Copy link
Contributor

Desiment commented Nov 8, 2025

  1. It seems more appropriate to add support property to Distribution protocol (note that support is mathematical terminology, not support object)
  2. I guess current constraint system is not optimal in a sense that a lot of things hardcoded to distribution type. On other the other hand we want keep constraint system non-public. This is the problem we need to solve, you suggested something nera Visitor pattern

Copy link
Contributor

@Desiment Desiment left a comment

Choose a reason for hiding this comment

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

Add docstrings please
Also, about constraints - I missed how our discussion about distribution type and constrains resolved. From what I see nothing changed

if np.any(vals <= 0.0):
return float("-inf")
return float(np.sum(np.log(vals)))
def log_likelihood(self, batch: Sample) -> float: ...
Copy link
Contributor

Choose a reason for hiding this comment

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

We decided to remove this method?

s = 0.0
for k in support.iter_leq(x):
s += float(pmf_func(float(k)))
return float(np.clip(s, 0.0, 1.0))
Copy link
Contributor

Choose a reason for hiding this comment

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

For distributions with infinite support, this method will not work. For example, if X, Y ~ Poiss(1), are independent, then X - Y has infinite support (that is there infinite amount of points in support.iter_leq(x)).

pass

# 3) Cursor-like API: first(), next(x)
if hasattr(support, "first") and hasattr(support, "next"):
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, "first" may not exists. As well as the "last".

X ~ Poiss(1) - last not exisits, support is N
Y ~ -Poiss(1) - first not exists, support is -N
X - Y support is Z

----------
distribution : Distribution
**options : Any
Unused (kept for a uniform API with continuous fitters).
Copy link
Contributor

Choose a reason for hiding this comment

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

But API for continuous fitters has an option for "left/right" ppf

Semantics
---------
``CDF(x) = sup { q ∈ [0,1] : PPF(q) ≤ x }``
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, this depends on the definition of PPF, and how it is computed (as left or right). Note that this does not matter in contiuous case (or, at least, should not)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants