Add missing `fix.family.*` functions

[dill]

Add in those fix.family.* methods which are not currently implemented in mgcv per Matteo's suggestion.

[mfasiolo]

I guess that the thing to do here would be to create a fixFamily or fixfamily set of functions, to avoid masking mgcv's functions. What do you think?

[gavinsimpson]

@mfasiolo What extra fix.family.xxx() functions were you thinking of? Or did you mean making sure that the fix.family.xxx() functionality applied to more families?

Also agree that we should have a separate function that calls the relevant fix.family.xxx if we know mgcv handles a family and if not we fill in with our own code, just as with simulate.gam().

My personal preference is fix_family_xxx(), but that is driven more by a growing appreciation for the tidy stuff and from having worked on gratia so much recently...

[mfasiolo]

@gavinsimpson I meant that we should produce a set of fix.family. function that covers all the families currently not covered by mgcv's version of fix.family.. I was thinking mainly about fix.family.rd, fix.family.qf() and fix.family.var() which seem the most useful for model checking?

Yes, we could do something like:

fix_family_rd <- function(fam){

  fam <- fix.family.rd(fam)

  # Try if mgcv provides $rd slot...
  if( !is.null(fam$rd) ) return( fam )

  # ... if not provide gamUtils' version
  fnam <- paste0(".rd.", fam$family)

  fam$rd <- get(fnam)

  return( fam )

}

# Internal to gamUtils?
.rd.gevlss <- function(mu, wt, scale) {
  return(mu[ , 1] + exp(mu[ , 2]) * 
        ((-log(runif(nrow(mu))))^(-mu[ , 3])-1) / mu[ , 3])
}

# Try it out
library(mgcViz)
dat <- gamSim(1,n=400,dist="normal",scale=2)
b <- gam(list(y~s(x0),~s(x1),~1),data=dat,family=gevlss)

b$family <- fix_family_rd(b$family)
head( simulate(b) )

I'm happy with the _ separated version as well.

[dill]

I was thinking more along the lines of this approach @mfasiolo. Even having our own fix.family.rd as a way to ensure that everything works "automatically", falling back to mgcv::fix.family.rd internally where necessary. I'm aware I am not thinking of potentially headaches with this namespace clash later though...

[mfasiolo]

@dill do you mean defining a new fix.family.rd, hence masking mgcv::fix.family.rd?

[dill]

Yes, that was what I was wondering. Not sure if this is a good idea though...

My understanding is that this would not affect operation of mgcv internals but may have knock-ons for mgcVix/gratia et al?

On 26/06/2020 14:16, Matteo Fasiolo wrote:

@dill https://github.com/dill do you mean defining a new |fix.family.rd|, hence masking |mgcv::fix.family.rd|?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dill/gamUtils/issues/5#issuecomment-650173123, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAPIIZQ3COMLDANTCGHUDRYSNRLANCNFSM4OGUJF5Q.

[mfasiolo]

Ok, maybe let's go for fix.family.rd, then we can check whether mgcViz or gratia brake.

[gavinsimpson]

I'll argue against this; I think masking is a really bad idea without a very good reason to do so. We shouldn't break much if packages have been using namespaces correctly (gratia will continue to work just fine because I import mgcv::fix.family.rd() for example) but as fix.family.xx are exported from the mgcv namespace we risk breaking peoples code that used the unqualified function name in their scripts unless we are very careful.

There's also the ambiguity of what fix.family.xx means? Is it the S3 method for classxx, class family.xx or something else? As such, . in function names are now discouraged, and if we want some consistency across this particular package, perhaps we want to use a different naming convention?

I suggest we have our own functions fix_family_xx() that use mgcv::fix.family.rd() where needed.

Alternatively, suggest to Simon to update mgcv and give him our code to do this. You both are much closer to Simon than I so you'll know how easy this would be to get done.

[mfasiolo]

Yeah, I agree on the masking issue (I just pushed fix.family.rd, but that's an easy fix). There is nothing to be gained by masking, as mgcViz (like gratia) imports mgcv::fix.family.rd anyway.

[dill]

Ah I see, thanks for the explanation Gavin. I think that's a good point and if it doesn't help but only (potentially) hurts let's avoid it.

Thanks for pushing the code Matteo!

Re: sending code to Simon, I'm not sure what his priorities are at the moment but I've not received responses from him on a few things so I assume he's snowed-under at the moment. We could consider a patch in the future for sure but I think this is the best mechanism for now.

On 26/06/2020 17:22, Matteo Fasiolo wrote:

Yeah, I agree on the masking issue (I just pushed fix.family.rd, but that's an easy fix). There is nothing to be gained by masking, as mgcViz (like gratia) imports mgcv::fix.family.rd anyway.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dill/gamUtils/issues/5#issuecomment-650268602, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAPILWITCBZ4AANBXZGZ3RYTDLXANCNFSM4OGUJF5Q.