cvitolo / fuse Goto Github PK
View Code? Open in Web Editor NEWfuse: an R package implementing the Framework for Understanding Structural Errors
Home Page: https://cvitolo.github.io/fuse/
fuse: an R package implementing the Framework for Understanding Structural Errors
Home Page: https://cvitolo.github.io/fuse/
This issue is part of this JOSS review
The README states install.packages(c("DBI", "assertthat", "magrittr", "tibble", "maptree", "tgp", "zoo", "qualV"))
But from DESCRIPTION the dependencies are
Imports: dplyr, zoo, tgp, stats
LinkingTo: BH, Rcpp
Suggests: testthat
Should the installation instructions be updated?
That said, I had no issue installing the package and am now running R CHECK 🎉
This issue is part of this JOSS review
This is again only a suggestion and quite similar to the suggestions made for the other datasets so I should maybe have grouped them, sorry.
parameters could have a more specific name, e.g. fuse_parameters.
the column names could be even more explicit (but I don't know about usual hydrology abbreviation so maybe I'm wrong), e.g. tishape
could be topology_index_shape
Running a package check I get the following NOTE:
checking compiled code ... NOTE
File ‘fuse/libs/fuse.so’:
Found ‘_ZSt4cout’, possibly from ‘std::cout’ (C++)
Object: ‘fuse.o’
File ‘fuse/libs/fuse.so’:
Found no calls to: ‘R_registerRoutines’, ‘R_useDynamicSymbols’
Compiled code should not call entry points which might terminate R nor
write to stdout/stderr instead of to the console, nor use Fortran I/O
nor system RNGs.
It is good practice to register native routines and to disable symbol
search.
Any idea how I could fix this?
I have added OSX to the list of operating systems to test on TRAVIS-CI, but it errors.
I do not have a mac, so any help to fix this is greatly appreciated!
This issue is part of this JOSS review
The title of the doc of fuserouting.ranges
is "Function to define the parameter ranges for GAMMA routing module" but the function has no parameter, it only returns the default parameters so it's not for defining something.
I'd propose to change the title to "default parameter ranges" or something like that.
maybe the name of the function could even contain the word "default"?
This issue is part of this JOSS review
I'd recommend adding a NEWS file to the package (devtools::use_news_md()
will create the template) for keeping track of changes introduced in the different version.
This issue is part of this JOSS review
Why not have the random seed as argument of fusesma.sim
and fuserouting.sim
and fuse
(or of fuse
only since it calls both other functions), with a default value, to make results reproducible even if the user forgets about the seed?
It would be great to use BH package rather than having code in the src folder
This issue is part of this JOSS review
These are only suggestions and questions:
I'd be in favour of naming modlist
more explicitely, e.g. fuse_models
, and maybe even without "list" in the name since it's a data.frame
(I understand the reason for calling it list, but I guess that because list is an object class in R it might be confusing?).
I do not understand the use of modliststring
. When looking for its occurrences in the repo I could only find the documentation. If the only role of modliststring
is to explain the codes, why are abbreviations used? Maybe it'd make sense to no longer have modliststring
and instead have modlist
and a data.frame per type of code with a column "code" and a column "meaning", or to have only modlist
and its documentation. Or to have only modliststring
and the documentation, i.e. use the abbreviations instead of the codes, and explain the abbrevations in the documentation?
The names of the variables themselves could be a bit more explicit, e.g. esoil
could be soil_evaporation
. Unless those are all abbreviations commonly used in hydrology?
I have followed the instruction given in the README.md file. I have installed all the prerequisite packages also. But still i'm getting error at the end of installation.
The Error message is like this! I need some help regarding this issue. I'm using R version of 3.6.3
C:/RBuildTools/3.5/mingw_32/i686-w64-mingw32/include/c++/bits/random.h:2504:11: note: template class std::gamma_distribution
class gamma_distribution
^
fuse.cpp:1722:24: error: expected primary-expression before '>' token
gamma_distribution<> gd(shape);
^
fuse.cpp:1722:34: error: 'gd' was not declared in this scope
gamma_distribution<> gd(shape);
^
make: *** [C:/PROGRA~1/R/R-3.6.3/etc/i386/Makeconf:215: fuse.o] Error 1
ERROR: compilation failed for package 'fuse'
removing 'C:/Users/maniv/Documents/R/win-library/3.6/fuse'
Error: Failed to install 'fuse' from GitHub:
(converted from warning) installation of package ‘C:/Users/maniv/AppData/Local/Temp/Rtmp677Gkt/file26b027a519f1/fuse_3.0.0.tar.gz’ had non-zero exit status
This issue is part of this JOSS review
I read in the README (& paper) "Implementation of the framework for hydrological modelling FUSE described in Clark et al. (2008) and based on the Fortran code provided by M. Clark in 2011. "
I'm absolutely not a licensing expert, so I just wonder whether the original Fortran code was released (and if so under which license -- but I guess it wasn't released since you mention it was kindly provided?), whether the author agreed to its use for a R package with the license you've chosen? Or maybe it doesn't play a role since your code is not a binding but rather a translation (and even improvement?) of the original code? Had you considered adding him as a contributor of the package? But I see DESCRIPTION too mention the original paper so this issue is probably useless. :-)
This issue is part of this JOSS review
I wonder if the name of the function fuse
is too confusing. Maybe you could rename it to something with "simulation" or "sim" in the name?
This issue is part of this JOSS review
This is only a suggestion: many exported functions of the package have a name starting with "fuse", why not all of them?
This issue is part of this JOSS review
Looking at codecov.io, it seems that the test coverage could be improved with a few more tests. :-)
This issue is part of this JOSS review
Could you add a "similar packages" section to the README and vignette to mention the CRAN Task View environmetrics https://cran.r-project.org/web/views/Environmetrics.html and whether some packages do something similar or whether some packages have an output that could be used with fuse
? This would help see how unique fuse
is and whether there are alternative/complementary packages in R.
When you submit the package to CRAN, I'd encourage you to contact the Task View maintainer to get fuse
added to it but you might have already thought of this anyway.
This issue is part of this JOSS review
Because in that case you might want to mention that as example somewhere in the documentation?
This issue is part of this JOSS review
The most important remark of this issue: the doc states DATA is a data.frame but it's actually a zoo
object, could you update the doc or the object itself?
These are details and only suggestions:
The name "DATA" is not very specific, why not name the sample data fuse_hydrological_data or something like that? If it's a time series, maybe even fuse_hydrological_timeseries?
The names of the columns/variables could be replaced by the full name of the variables, e.g. "precipitation" instead of P.
This issue is part of this JOSS review
A cache issue?
This issue is part of this JOSS review
The doc of fuserouting.sim should mention the class of the arguments, e.g. whether deltim
is a numeric.
This issue is part of this JOSS review
I actually get an error now when running the tests,
1. Failure: Ensemble run (@test-fuse.R#54) -------------------------------------
all(discharges == y) not equal to TRUE.
1 element mismatch
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.