rvw-org / rvw

R interface to Vowpal Wabbit
22 stars 2 forks source link

Add readable model and quiet execution #6

Closed ivan-pavlov closed 6 years ago

eddelbuettel commented 6 years ago

Looks good at a first glance. Some warnings, I'll report some more later.

eddelbuettel commented 6 years ago

Some more from here:

  1. Alter License: in DESCRIPTION as we have
* checking top-level files ... NOTE
File
  LICENSE
is not mentioned in the DESCRIPTION file.
* checking for left-over files ... OK

Any one GPL-2 or GPL (>= 2) or ... that is compatiable with VW should work. We include little to no existing code so GPL should really work.

  1. Change code as g++ version 7 picks this up on possibly misleading indentation:
rvw.cpp: In function ‘void create_cache(std::__cxx11::string, std::__cxx11::string, std::__cxx11::string)’:
rvw.cpp:25:5: warning: this ‘while’ clause does not guard... [-Wmisleading-indentation]
     while ( cache_model->early_terminate == false )
     ^~~~~
rvw.cpp:31:9: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘while’
         if (cache_model->early_terminate) //drain any extra examples from parser.
         ^~
ccache g++ -std=gnu++11 -Wl,-S -shared -L/usr/lib/R/lib -Wl,-Bsymbolic-functions -Wl,-z,relro -o rvwgsoc.so RcppExports.o helpers.o rvw.o -lvw -L/usr/lib/R/lib -lR
installing to /usr/local/lib/R/site-library/rvwgsoc/libs
  1. Look into this please -- dual caches created ?
R> example(vwsetup)                                 

vwsetpR> vwsetup(                                   
vwsetp+  dir = tempdir(),                           
vwsetp+  train_data = "X_train.vw",                 
vwsetp+  test_data = "X_valid.vw",                  
vwsetp+  model = "pk_mdl.vw",                       
vwsetp+  cache = TRUE,                              
vwsetp+  general_params = list(passes=10),          
vwsetp+  optimization_params = list(adaptive=FALSE),
vwsetp+  learning_params = list(binary=TRUE)        
vwsetp+ )                                           
Num weight bits = 18                                
learning rate = 0.5                                 
initial_t = 0                                       
power_t = 0.5                                       
creating cache_file = /tmp/RtmpRfOvVP/X_train.vw.cache
Warning: you tried to make two write caches.  Only the first one will be made.
Reading datafile = /tmp/RtmpRfOvVP/X_train.vw       
can't open '/tmp/RtmpRfOvVP/X_train.vw', sailing on!
num sources = 0                                     

finished run               
[...]
eddelbuettel commented 6 years ago

Looks good; R CMD check is now clean, and the examples run -- nice! We got ourselves a great baseline to now do more exciting ML with VW from!

One minor style point: first lines on git commits are usually capped which means displays of the log do look nicer -- git ls is a standard alias. Your most recent commit put an essay in the first line, so it breaks.

For that one can use lines 3 and onwards (leaving line 2 empty) or maybe use a NEWS, NEWS.Rd, NEWS.md, ... file or a ChangeLog. Just a thought.

edd@rob:~/git/rvwgsoc(feature/readable-and-quiet-outputs)$ git ls
* 9fd1e2a - (HEAD -> feature/readable-and-quiet-outputs, origin/feature/readable-and-quiet-outputs) Add descriptions for vwtrain, vwtest and vwsetup functions. Delete License file to fix DESCRIPTION file. Change code indention in create_cache function. Fix dual caches creation. Caches are now created in the correct folder. (38 minutes ago) <Ivan Pavlov>
* 45bd826 - Add tests, demo and documentation. (17 hours ago) <Ivan Pavlov>
* c0ee9ee - Add invert_hash and readable model outputs. Add quiet vwtrain and vwtest execution. (2 days ago) <Ivan Pavlov>
* a55c118 - (origin/master, origin/HEAD, master) Address merge conflict (#5) (3 days ago) <James J Balamuta>