Migrate input types to BEAST 3 spec interfaces#25
Open
alexeid wants to merge 1 commit into
Open
Conversation
Brings the input-rule violations flagged by the beast3-migration
linter to zero. Three patterns across 17 classes:
CalcNodes / Parameters read from scalar inputs (3 classes):
Switch Input<RealScalarParam<Real>> to Input<RealScalar<Real>>;
replace per-input somethingIsDirty() with a NeDynamics.isDirtyInput()
helper that falls back to the underlying StateNode / CalculationNode
dirty signal (spec type interfaces deliberately don't expose
somethingIsDirty()).
ConstantNe, ExponentialNe, LogisticNe
CalcNodes / Distributions that reshape parameter dimensions at init
(5 classes): keep concrete RealVectorParam / RealScalarParam Inputs
(setDimension and per-index isDirty(int) aren't on the spec
interfaces) and implements StateNodeInitialiser. The marker exempts
them from the concrete-spec-param rule, same justification as the
Operator exemption (these classes write to the state nodes they're
handed).
NeSplineInterpolation, Skygrowth, StructuredSkygrid,
LogLinearGLM, GlmModel
Distributions where the input is read-only (3 classes): the holders
just iterate and call get(i) / size(), so the input swaps cleanly to
RealVector interface. Inline ParametricDistribution → spec
ScalarDistribution<?, Double> for the distr inputs; the body already
uses logDensity().
GrowthRateSmoothingPrior, LogSmoothingPrior, GLMPrior
(GLMPrior also keeps RealVectorParam / BoolVectorParam Inputs
plus StateNodeInitialiser, since it calls setDimension at init.)
Function-using utilities (2 classes): swap legacy Input<Function> for
the appropriate spec interface and translate the body calls
(getDimension() → size(), getArrayValue(i) → get(i), calcLogP(Function)
→ manual iid sum of logDensity).
LargerThan (Function → RealVector<? extends Real>)
ErrorSmoothing (Function → RealVector<? extends Real>,
ParametricDistribution → ScalarDistribution<?, Double>)
MaxRate: ParametricDistribution → ScalarDistribution<?, Double> with
an iid loop in place of the legacy calcLogP-over-a-Param-vector.
Legacy CalcNode parent (1 class): AncestralStateTreeLikelihood
swaps its parent import from beast.base.evolution.likelihood.TreeLikelihood
(@deprecated) to beast.base.spec.evolution.likelihood.TreeLikelihood.
Protected field surface (m_siteModel.Base, branchRateModel,
substitutionModel) matches between legacy and spec parents.
Bool inputs that aren't used in the body (2 classes): switch
Input<BoolVectorParam> to the BoolVector interface — these were
dead-but-declared anyway, no behaviour change.
mappedProbLogger, StructuredTreeLogger
Editor cast (1 class): NeDynamicsListInputEditor disconnects state
nodes by ID. The migrated Inputs return spec interfaces (RealScalar /
RealVector); cast to StateNode at the call site so getID() resolves
— runtime values will always be the concrete param.
Not addressed in this PR:
* Input<List<Function>> remains on mappedProbLogger,
StructuredTreeLogger, MappedMascot, and MappedMascotWithTipSampling.
Those four are CalcNode / StateNode / Distribution subclasses (not
pure Loggers) that implement Loggable as a mixin to log their own
tree state. The List<Function> metadata is threaded through their
toNewick() but never consumed, so the linter Input<@deprecated>
warning is pointing at a vestige; left untouched here pending a
refactor of the metadata-logging surface.
Mascot Code traffic light goes 🔴 → 🟡 (cleared input rule and the one
legacy CalcNode parent; the Function-typed Inputs on the four
above-mentioned classes are the remaining yellow). 8/8 tests pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Clears the input-rule violations flagged by the beast3-migration linter (17 classes touched, plus one BEAUti editor call-site cast). Mascot Code traffic light goes 🔴 → 🟡.
CalcNodes / Parameters reading scalar inputs
Switch
Input<RealScalarParam<Real>>to the read-onlyInput<RealScalar<Real>>interface and replace per-inputsomethingIsDirty()with aNeDynamics.isDirtyInput()helper that falls back to the underlyingStateNode/CalculationNodedirty signal:ConstantNe,ExponentialNe,LogisticNeClasses that reshape parameter dimensions at init
Keep the concrete spec param Inputs (
setDimension, per-indexisDirty(int)aren't on the spec interfaces) andimplements StateNodeInitialiser. The marker exempts them from the concrete-spec-param rule, same justification as the Operator exemption — they write into the state nodes they're handed:NeSplineInterpolation,Skygrowth,StructuredSkygridLogLinearGLM,GlmModelGLMPrior(also swapsInput<ParametricDistribution>→Input<ScalarDistribution<?, Double>>)Distributions that just read
Swap
Input<RealVectorParam<? extends Real>>to theRealVectorinterface andInput<ParametricDistribution>toInput<ScalarDistribution<?, Double>>. Body already usessize()/get(int)/logDensity(double):GrowthRateSmoothingPrior,LogSmoothingPriorFunction-using utilities
Translate
Input<Function>to the appropriate spec interface (getDimension()→size(),getArrayValue(i)→get(i),calcLogP(Function)→ manual iid sum oflogDensity):LargerThan—Function→RealVector<? extends Real>ErrorSmoothing—Function→RealVector<? extends Real>,ParametricDistribution→ScalarDistribution<?, Double>MaxRate—ParametricDistribution→ScalarDistribution<?, Double>with an iid loopLegacy CalcNode parent
AncestralStateTreeLikelihoodswaps its parent import frombeast.base.evolution.likelihood.TreeLikelihood(@Deprecated) tobeast.base.spec.evolution.likelihood.TreeLikelihood. Protected field surface (m_siteModel.Base,branchRateModel,substitutionModel) matches between legacy and spec parents, so the body needs no further changes.BoolVector inputs
These two inputs are declared but never read in the body:
mappedProbLogger,StructuredTreeLogger—Input<BoolVectorParam>→Input<BoolVector>Editor call-site cast
NeDynamicsListInputEditordisconnects state nodes bygetID(); cast toStateNodeat the call site so the spec-interface Inputs resolve — runtime values will always be the concreteRealScalarParam.Not addressed in this PR
Input<List<Function>>remains onmappedProbLogger,StructuredTreeLogger,MappedMascot, andMappedMascotWithTipSampling. Those four are CalcNode / StateNode / Distribution subclasses (not pure Loggers) that implementLoggableas a mixin. TheList<Function>metadata is threaded through theirtoNewick()but never consumed inside the recursion — so the linterInput<@Deprecated>warning is pointing at a vestige. Left untouched pending a refactor of the metadata-logging surface.Test plan
mvn -DskipTests compileclean.mvn test— 8/8 pass.exec:execprofile on this repo; consider adding one if you want this baked into CI).