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

Dirichlet family #60

Open
wants to merge 85 commits into
base: devel
Choose a base branch
from
Open

Dirichlet family #60

wants to merge 85 commits into from

Conversation

Tanzmarie
Copy link

Dear gamboostLSS-Team,

I have implemented a novel Dirichlet family which allows for fitting Dirichlet regression models via gradient boosting within your gamboostLSS package for a paper that is currently under review. One referree pointed out that it would be favorable to add the family to gamboostLSS. To this end, I adjusted the necessary file in my fork and conducted relevant tests.

I would be really happy if you would consider adding the Dirichlet family.

Kind regards,
Michael

@hofnerb
Copy link
Member

hofnerb commented Jan 6, 2025

@mayrandy, it seems that our devel branch is completely outdated and that Michael (@Tanzmarie) has used that branch to develop his family. I am not sure if one can still change the merge target to master and check for differences there? I am currently having issues reviewing the code as it is based on many changes from the devel branch.

As a sidenote: We should consider what to do with the devel branch. It seems that we should either update it to the most current version or remove it completely to avoid issues. Maybe we can have a short call in the comming days?

@hofnerb
Copy link
Member

hofnerb commented Jan 6, 2025

@Tanzmarie: The code is currently not ok. Please run R checks and remove all notes, warnings and errors related to your code. A quick check gives me the following issues:

  • DirichletAlpha: no visible binding for global variable 'loss'
    Undefined global functions or variables:
    loss

  • checking Rd line widths ... NOTE
    Rd file 'families.Rd':
    \usage lines wider than 90 characters:
    DirichletLSS(K = NULL, a1=NULL, a2=NULL, a3=NULL, a4=NULL, a5=NULL, a6=NULL, a7=NULL, a8=NULL, a9=NULL,
    m1=NULL, m2=NULL, m3=NULL, m4=NULL, m5=NULL, m6=NULL, m7=NULL, m8=NULL, m9=NULL,
    s1=NULL, s2=NULL, s3=NULL, s4=NULL, s5=NULL, s6=NULL, s7=NULL, s8=NULL, s9=NULL,

    These lines will be truncated in the PDF manual.

  • checking for code/documentation mismatches ... WARNING
    Codoc mismatches from Rd file 'families.Rd':
    DirichletLSS
    Code: function(K, a1 = NULL, a2 = NULL, a3 = NULL, a4 = NULL, a5 =
    NULL, a6 = NULL, a7 = NULL, a8 = NULL, a9 = NULL, m1 =
    NULL, m2 = NULL, m3 = NULL, m4 = NULL, m5 = NULL, m6 =
    NULL, m7 = NULL, m8 = NULL, m9 = NULL, s1 = NULL, s2 =
    NULL, s3 = NULL, s4 = NULL, s5 = NULL, s6 = NULL, s7 =
    NULL, s8 = NULL, s9 = NULL, ...)
    Docs: function(K = NULL, a1 = NULL, a2 = NULL, a3 = NULL, a4 = NULL,
    a5 = NULL, a6 = NULL, a7 = NULL, a8 = NULL, a9 = NULL,
    m1 = NULL, m2 = NULL, m3 = NULL, m4 = NULL, m5 = NULL,
    m6 = NULL, m7 = NULL, m8 = NULL, m9 = NULL, s1 = NULL,
    s2 = NULL, s3 = NULL, s4 = NULL, s5 = NULL, s6 = NULL,
    s7 = NULL, s8 = NULL, s9 = NULL, ...)
    Mismatches in argument default values:
    Name: 'K' Code: Docs: NULL

  • checking Rd \usage sections ... WARNING
    Undocumented arguments in Rd file 'families.Rd'
    'K' 'a1' 'a2' 'a3' 'a4' 'a5' 'a6' 'a7' 'a8' 'a9' 'm1' 'm2' 'm3' 'm4'
    'm5' 'm6' 'm7' 'm8' 'm9' 's1' 's2' 's3' 's4' 's5' 's6' 's7' 's8' 's9'

    Functions with \usage entries need to have the appropriate \alias
    entries, and all their arguments documented.
    The \usage entries must correspond to syntactically valid R code.
    See chapter 'Writing R documentation files' in the 'Writing R
    Extensions' manual.

  • checking for unstated dependencies in 'tests' ... WARNING
    'library' or 'require' call not declared from: 'DirichletReg'

  • Running the tests in 'tests/regtest-families.R' failed.
    Last 13 lines of output:

    n <- 150
    p <- 10

    x <- matrix(runif(p * n, 0,1), n)

    x <- data.frame(x)

    a1 <- exp(2.5x[,1] - x[,2] + 3x[,3])
    a2 <- exp(2x[,4] + 2x[,5] - x[,6])
    a3 <- exp(1.5x[,7] - 1.5x[,8] + x[,9])
    A <- cbind(a1,a2,a3)

    y <- rdirichlet(nrow(A),A)
    Error in rdirichlet(nrow(A), A) : could not find function "rdirichlet"
    Execution halted

If possible you could also use the master branch and add your code to that to make the pull request easier as discussed above. I am also wondering if you really need the lengthy lists of arguments (a1, ...) which I currently don't even understand given a lack of documentation. Maybe we can see if it possible to use a vector or list of arguments instead?

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.

4 participants