JuliaAI / JuliaAI/MLJLinearModels.jl

Use AverageLoss

Open
#110 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
Julia
Stars
86
Forks
15
PR merge metrics
No merged PRs in 30d

Description

By the way, when I started working on this PR I considered for a moment an alternative implementation with a wrapper

struct AverageLoss{L} <: Loss
    loss::L
end

(al::AverageLoss)(x::AVR, y::AVR) = al.loss(x, y)/length(y)

I found this pretty neat, but it would have required some major refactoring, mostly because of the parametric dispatch on glr for _solver, _fit etc. Are all these type restrictions really needed? Or could one get rid of them, maybe with a few traits?

But I am also fine with the current implementation. Please feel free to merge whenever you feel this PR is ready.

Originally posted by @jbrea in https://github.com/JuliaAI/MLJLinearModels.jl/issues/109#issuecomment-1004035779

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading issue 109 and the referenced pull request, then inspect the _solver and _fit dispatch mentioned here. Compare the proposed AverageLoss wrapper with the current implementation and determine whether the type restrictions can be removed or replaced with traits. The issue does not define a settled implementation or completion criteria.

Written by the indexing model from the issue text.

Assessment

Tech stack
julia
Domain
machine-learning
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.