Rate statistic - #120
Conversation
alexeid
left a comment
alexeid
left a comment
There was a problem hiding this comment.
Looks okay. I understand this it to make RateStatistic a RealVector? But when is that actually needed? It just goes into a logger normally doesn't it? So it can still be a function since loggers aren't part of the model and strong typing only needs to be enforced for model components?
Because it uses a deprecated class, which was replaced by strong typing when tracing the inheritance chain. In addition, I presume |
|
Removing the old RateStatistic means old XML cannot run any more. |
the old (deprecated) RateStatistic would not be removed, unless |
The statistic exposes mean, variance and coefficient of variation through one RealVector domain, so that domain has to admit every value the three can take. Variance and coefficient of variation are exactly 0 whenever the branches share a rate, which is the ordinary state of affairs under a strict clock, and PositiveReal excludes 0 (lowerInclusive() is false): strict clock: mean=1.0 variance=0.0 cv=0.0 isValid() over all 3 : false Nothing in parser, core or inference calls isValid() yet, so this is latent rather than a crash today -- but the spec migration plainly intends that enforcement, and by then the class will have shipped and been adopted. NonNegativeReal still rejects negatives, which none of the three statistics can be. Adds RateStatisticTest: the strict-clock case (fails against PositiveReal), that negatives are still rejected, the weighted-mean and unweighted-variance values for varying rates, and the index guard on get() that this migration tightened from the legacy `dim > 3` check. beast-base suite: 462 tests, 0 failures.
Widen RateStatistic domain to NonNegativeReal
3 participants
migrate RateStatistic and update xml, module-info, and version.xml.
see #119