famuvie / breedR

Statistical methods for forest genetic resources analysts
http://famuvie.github.io/breedR/
GNU General Public License v3.0
31 stars 24 forks source link

make ranef() return cond. mode of generic random variables with their names #87

Closed timflutre closed 6 years ago

timflutre commented 7 years ago

I am using the generic argument of remlf90() like this:

generic=list(geno=list(incidence=Z, covariance=G, var.ini=2))

in which the genotype names are given as column names of Z and as row and column names of G.

However, when I am using ranef(), its output doesn't have any of the input genotype name (neither the vector of conditional modes nor the "se" attribute). More specifically, the vector of conditional modes has no name, whereas the "se" attribute has names but they don't correspond to the input genotype names provided in Z and G. Instead they correspond to integer indices.

I could provide a pull request, but can you confirm me that for that I should only modify ranef.remlf90()? For instance, after ans <- get_estimates(object$ranef), one could add names of generic random variables (from, say, the columns of the covariance matrix, here G), if any is provided. What do you think?

famuvie commented 6 years ago

Issue confirmed, thanks.

I guess you can do as you say. Adding a specific chunk of code into ranef.remlf90() that gathers names from generic effects in the model.

I'm willing to accept a PR with this approach in order to solve the issue quickly, although I feel that a better way would be to record the names directly in the object$ranef at the moment of constructing the resulting object, within parse_results(). This would simplify considerably ranef.remlf90() rather than having to gather names one way or another depending on the specifics of the model component (genetic, spatial, generic, ...). But that would be more complicated for you to implement.

So, you sweep object$effects looking for effects inheriting from generic, and gather the row/column names from either the incidence or structure matrices. The little annoyance is that you have to rely on variable effect names (rather than genetic or spatial which are fixed within breedR).

Hope it helps.

timflutre commented 6 years ago

Ok, I will try to look into that. Thanks for your reply!

timflutre commented 6 years ago

In terms of workflow, should I modify directly the "dev" branch to be able to propose pull request on this branch? Or should you first create a branch named, say "names_generic", and then I modify it and do a pull request on it?

famuvie commented 6 years ago

Fork the repo and create the feature-branch "names_generic" yourself in your fork. Then PR into master directly. (see https://github.com/famuvie/breedR/wiki/Developing-breedR#git-branching-workflow)

Thanks for asking ;)

timflutre commented 6 years ago

I wrote some code and now would like to run all unit tests (as specified in your guidelines linked in a previous post).

So, in a terminal, here is what I do:

$ cd ~/src/breedR
$ Rscript -e 'library(devtools); devtools::test()'

But I get the following error:

Loading breedR
Loading required package: testthat                                                                                                                                              
Loading required package: sp                                                                                                                                                    
Loading required package: methods                                                                                                                                               
Testing breedR                                                                                                                                                                  
Building additive-genetic models: .......                                                                                                                                       
AR infrastructure: ..................                                                                                                                                           
Blocks infrastructure: .......                                                                                                                                                  
Checks for model components: Error in breedR.bin.builtin() :                                                                                                                    
  breedR installation error; no such directory                                                                                                                                  
Calls: <Anonymous> ... eval -> eval -> breedR.getOption -> breedR.bin.builtin                                                                                                   

DONE ===========================================================================                                                                                                
Execution halted                                                                                                                                                                

What should I do? (Note that I also have the latest release of breedR already properly installed.)

timflutre commented 6 years ago

I looked at the files utils.R and development.R but couldn't manage to find a solution to the error reported above. Even though executing breedR:::breedR.bin.builtin() works without error in an R console, it fails when running devtools.

famuvie commented 6 years ago

Hi Tim, sorry for the delay. You need to have the binaries installed in your source tree as well (within inst/bin), which you probably don't. You can just make a soft-link to the binaries in your installed breedR package.

Let me know whether this is it.

timflutre commented 6 years ago

No problem. Thanks, it is now solved! I think this could be added to the wiki page "Developing breedR".

famuvie commented 6 years ago

Absolutely! You are probably the first person other than me doing this :) Thanks.

timflutre commented 6 years ago

;)

timflutre commented 6 years ago

I made a pull request but the continuous integration reports an error (which has nothing to do with what I changed).

famuvie commented 6 years ago

Cool, thanks!

I also took the opportunity to improve the Developing breedR wiki page.

timflutre commented 6 years ago

Thanks!

I just fixed a few typos on the wiki (it happens that anyone can edit wiki pages).

famuvie commented 6 years ago

Super, thanks. Note that these changes are not available in the "released" version of breedR (i.e. the one that you install using install.packages from breedR's repository). You can install locally from your development version using devtools::install().

timflutre commented 6 years ago

yes, it's only in the master branch for now, no problem