Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace built-in measures with measures in StatisticalMeasures.jl #909

Merged
merged 14 commits into from
Sep 21, 2023

Conversation

ablaom
Copy link
Member

@ablaom ablaom commented May 25, 2023

In this PR we:

  • (breaking) Remove the definition of all measures (mae, log_loss, etc), which can instead be loaded from StatisticalMeasures.jl, which provides like-named measures, with the exception of those previously wrapped from LossFunctions.jl, which now must be explicitly imported and wrapped. (Julia >= 1.9). Breaking behaviour and workarounds are detailed below. For Julia < 1.9, make StatisticalMeasures a hard dependency and re-export it's names. For Julia 1.9 provide default measures for models using a Pkg extension, including StatisticalMeasures as a weak dependency.

  • Remove the LossFunctions.jl dependency to close Investigate loading time of LossFunctions.jl #570.

  • Add StatisticalMeasures.jl as a weak dependency, so that default measures can be provided through an Pkg extension, ext/DefaultMeasuresExt.jl.

  • Adapt the resampling (evaluate!) code (and a semi-duplication for model Stacks) to the new measure API set out in StatisticalMeasuresBase.jl.

This PR closes #502, as StatisticalMeasures.jl provides some out-of-the-box multi-target measures and StatisticalMeasuresBase.jl provides a wrapper multimeasure to get more. Similarly, closes #529.

Breaking behaviour relevant to many users

  • In Julia 1.9 or higher, StatisticalMeasures.jl must be explicitly imported to use measures that were previously part of MLJBase (but new releases of MLJ will import StatisticalMeasures.jl, so they will be available that umbrella pkg).

  • Measures that in MLJBase skipped NaN values will now (at least by default) propagate those values, when imported from StatsticalMeasures.jl. Missing value behaviour is unchanged, except some measures that previously did not support missing now do.

  • All measures return a single aggregated measurement. In other words, measures
    previously reporting a measurement per-observation (previously subtyping
    Unaggregated) no longer do so. To get per-observation measurements, use the new method
    measurements(measure, ŷ, y[, weights, class_weights]) from StatisticalMeasures.jl.

  • Measures with a "feature argument" X, as in some_measure(ŷ, y, X), are no longer
    supported. See What is a
    measure?

    for allowed signatures in measures.

  • Aliases for measure types have been removed. For example RMSE (alias for
    RootMeanSquaredError) is gone. Aliases for instances, such as rms and
    cross_entropy have been preserved. The exception is precision, for which ppv can
    be used in its place. (This is to avoid conflict with Base.precision, which was
    previously pirated; you can also do StatisticalMeasures.precision.)

  • info(measure) has been decommissioned; query docstrings or access the new measure
    traits

    individually instead. These traits are now provided by StatisticalMeasures.jl and not
    re-exported.

  • Behaviour of the measures() method, to list all measures and associated traits, has
    changed. It now returns a dictionary instead of a vector of named tuples;
    measures(predicate) is decommissioned, but measures(needle) is preserved. (This method,
    owned by StatisticalMeasures.jl has some new search options.)

  • Measures that were wraps of losses from LossFunctions.jl are no longer exposed by
    MLJBase.jl. To use such a loss, you must explicitly import LossFunctions and wrap the
    loss appropriately. See Using losses from
    LossFunctions.jl

    for examples.

  • Some user-defined measures working in previous versions of MLJBase.jl may not work
    without modification, as they must conform to the new StatisticalMeasuresBase.jl
    API
    . See
    this tutorial on
    how define new measures.

  • The default measure for regression models (used in evaluate/evaluate! when measures is unspecified) is changed from rms to l2=LPLoss(2) (mean sum of squares).

Breaking behaviour likely relevant only to developers of some client packages

  • The abstract measure types Aggregated, Unaggregated, Measure have been
    decommissioned. (A measure is now defined purely by its calling
    behaviour
    .)

  • What were previously exported as measure types are now only constructors.

  • target_scitype(measure) is decommissioned. Related is
    StatisticalMeasures.observation_scitype which declares an upper bound on the allowed
    scitype of a single observation

  • prediction_type(measure) is decommissioned. Instead use
    StatisticalMeasures.kind_of_proxy.

  • The trait reports_each_observation is decommissioned. Related is
    StatisticalMeasures.can_report_unaggregated which is false when the new
    measurements method returns n copies of the aggregated measure, where n is the
    number of observations provided, instead of individual observation-dependent
    measurements.

  • aggregation(measure) has been decommissioned. Instead use
    StatisticalMeasures.external_mode_of_aggregation(measure).

  • instances(measure) has been decommissioned; query docstrings for measure aliases, or
    follow this example: aliases = measures()[RootMeanSquaredError].aliases.

  • is_feature_dependent(measure) has been decommissioned. Measures consuming feature data are not longer
    supported; see above.

  • distribution_type(measure) has been decommissioned.

  • docstring(measure) has been decommissioned.

  • Behaviour of aggregate has changed.

  • The following traits, previously exported by MLJBase, cannot be applied to measures:
    supports_weights, supports_class_weights, orientation, human_name. Instead use
    the traits with these names provided by StatisticalMeausures.jl (they will need to be
    qualified, as in using StatisticalMeasures; StatisticalMeasures.orientation(measure)).

@ablaom ablaom mentioned this pull request May 25, 2023
4 tasks
@ablaom ablaom marked this pull request as draft May 25, 2023 19:44
@ablaom ablaom closed this May 25, 2023
@ablaom ablaom reopened this May 25, 2023
@codecov-commenter
Copy link

codecov-commenter commented May 25, 2023

Codecov Report

Patch coverage: 95.91% and project coverage change: -1.55% ⚠️

Comparison is base (c6dddce) 87.78% compared to head (0549150) 86.23%.

Additional details and impacted files
@@                     Coverage Diff                      @@
##           for-a-0-point-22-release     #909      +/-   ##
============================================================
- Coverage                     87.78%   86.23%   -1.55%     
============================================================
  Files                            40       32       -8     
  Lines                          3659     2833     -826     
============================================================
- Hits                           3212     2443     -769     
+ Misses                          447      390      -57     
Files Changed Coverage Δ
ext/DefaultMeasuresExt.jl 66.66% <66.66%> (ø)
src/composition/models/stacking.jl 94.82% <88.88%> (-0.50%) ⬇️
src/resampling.jl 91.86% <98.43%> (+0.36%) ⬆️
src/MLJBase.jl 100.00% <100.00%> (ø)
src/default_measures.jl 100.00% <100.00%> (ø)
src/utilities.jl 84.39% <100.00%> (-0.42%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ablaom ablaom marked this pull request as ready for review May 26, 2023 20:13
@ablaom
Copy link
Member Author

ablaom commented May 30, 2023

I'm not expecting detailed review of this PR, but think someone should at least look over the planned breakages, and so forth.
@rikhuijzer, @OkonSamuel could you take a cursory look?

Copy link
Member

@rikhuijzer rikhuijzer left a comment

Choose a reason for hiding this comment

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

I don't understand all the nuances of this large change, but here are some extra thoughts:

  • Due to the large amount of changes, is this a nice moment to go to v1.0.0? Versions of 1 and up have the benefit that there is more control due to MAJOR.MINOR.PATCH instead of MAJOR.MINOR/PATCH. Also there is a risk of being mocked at https://0ver.org/ when staying at <1.0.0 😛
  • Should there also be a migration guide somewhere like in the docs? The PR description would be reasonable, but also contains non-essential information making it harder for clients to migrate.

# for julia < 1.9
if !isdefined(Base, :get_extension)
include(joinpath("..","ext", "DefaultMeasuresExt.jl"))
@reexport using .DefaultMeasuresExt.StatisticalMeasures
Copy link
Member

Choose a reason for hiding this comment

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

So StatisticalMeasures is only available in the public (re-exported) scope in Julia 1.9 or does the new extensions system from Julia automatically bring things to the public scope? (I'm not familiar with the new system yet)

Copy link
Member Author

Choose a reason for hiding this comment

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

If using MLJBase without MLJ, then, in Julia 1.9 or higher, StatisticalMeasures must be explicitly imported to use measures that were previously part of MLJBase (the names are automatically re-exported by a pkg extension). For earlier of Julia versions, StatisticalMeasures.jl is a hard dependency (through a hack that is standard practice). If using MLJ, then all previous measures are still available, because StatisticalMeasures.jl will be a hard dep of MLJ.

results = NamedTuple{modelnames}(
[(
measure = stack.measures,
measurement = Vector{Any}(undef, n_measures),
operation = _actual_operations(nothing, stack.measures, model, verbosity),
per_fold = [Vector{Any}(undef, nfolds) for _ in 1:n_measures],
per_observation = Vector{Union{Missing, Vector{Any}}}(missing, n_measures),
per_observation = [Vector{Vector{Any}}(undef, nfolds) for _ in 1:n_measures],
Copy link
Member

Choose a reason for hiding this comment

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

Just to double check, per_observation is now a Vector{Vector{Vector{Any}}}?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, per_observation[i][j][k] is the the measurement for the kth observation, in the jth fold, for the ith measure. There no longer can be missings because measures that don't compute per-observation measurements, simply report copies of the aggregated instead.

@rikhuijzer
Copy link
Member

Overall this PR looks like an improvement. The main idea was to make StatisticalMeasures.jl easier available to other packages, right? Maybe also good to clarify for users.

In general I'm in favor of merging this. It looks like a clear improvement.

@ablaom
Copy link
Member Author

ablaom commented Sep 21, 2023

Due to the large amount of changes, is this a nice moment to go to v1.0.0? Versions of 1 and up have the benefit that there is more control due to MAJOR.MINOR.PATCH instead of MAJOR.MINOR/PATCH. Also there is a risk of being mocked at https://0ver.org/ when staying at <1.0.0 😛

Version 1.0.0 it is.

Should there also be a migration guide somewhere like in the docs? The PR description would be reasonable, but also contains non-essential information making it harder for clients to migrate.

Good idea. https://github.com/alan-turing-institute/MLJ.jl/blob/measure/docs/src/performance_measures.md#migration-guide-for-changes-to-measures-in-mljbase-10

@ablaom ablaom merged commit 0ee7f41 into for-a-0-point-22-release Sep 21, 2023
4 of 5 checks passed
@ablaom ablaom deleted the measures branch September 21, 2023 23:44
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.

3 participants