changing parallel loops to Future.apply

[parismita]

issue #32

[parismita]

@tdhock I converted to future.apply but to use future::plan(), I need to import future, then it gives the following warning:

Warning message:
replacing previous import ‘future::future_lapply’ by ‘future.apply::future_lapply’ when loading ‘mmit’ 

But if I use future:future_lapply, that function is deprecated so it gives warning again.

What shall I do now?

[tdhock]

hi you should not use future::plan in your package

the user of your package uses future::plan to tell the system how to implement the parallel computation

On Mon, Feb 18, 2019 at 8:06 AM parismita notifications@github.com wrote:

@tdhock https://github.com/tdhock I converted to future.apply but to use future::plan(), I need to import future, then it gives the following warning:

Warning message:

replacing previous import ‘future::future_lapply’ by ‘future.apply::future_lapply’ when loading ‘mmit’

But if I use future:future_lapply, that function is deprecated so it gives warning again.

What shall I do now?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aldro61/mmit/pull/33#issuecomment-464765688, or mute the thread https://github.com/notifications/unsubscribe-auth/AA478vN0zIWecQDuRz5hE6xJlUm1_WE9ks5vOsFWgaJpZM4XvUjk .

[parismita]

ok, so the user will define the no of cpus as well as the implementation.....So shall I remove it?

[tdhock]

yes you should remove future::plan in your package code

but you need to keep the future.apply::future_lapply in your package.

On Mon, Feb 18, 2019 at 9:13 AM parismita notifications@github.com wrote:

ok, so the user will define the no of cpus as well as the implementation.....So shall I remove it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aldro61/mmit/pull/33#issuecomment-464793578, or mute the thread https://github.com/notifications/unsubscribe-auth/AA478qx2__fsaRJ3_dQi_aHab88SpNwXks5vOtESgaJpZM4XvUjk .

[parismita]

Will the user use makeCluster by itself?

[tdhock]

the user gets to decide how to do the parallelism, maybe the user will use parallel package / makeCluster, maybe the user will use something else...

On Mon, Feb 18, 2019 at 9:22 AM parismita notifications@github.com wrote:

Will the user use makeCluster by itself?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aldro61/mmit/pull/33#issuecomment-464797005, or mute the thread https://github.com/notifications/unsubscribe-auth/AA478mkkTlLnHFvU2AVt4qWsCMWjr8mxks5vOtMpgaJpZM4XvUjk .

[parismita]

ok...I'll remove them and add a note in the documentation regarding the same...that the user gets to decide how to do the parallelism

[tdhock]

great.

for an example of how I did it in another package see https://github.com/tdhock/penaltyLearning/blob/master/R/IntervalRegression.R#L99

a good idiom is

LAPPLY <- if(requireNamespace("future.apply")){ future.apply::future_lapply }else{ lapply }

so then future.apply is not a hard dependency (you should put it under Suggests in DESCRIPTION)

On Mon, Feb 18, 2019 at 9:34 AM parismita notifications@github.com wrote:

ok...I'll remove them and add a note in the documentation regarding the same...that the user gets to decide how to do the parallelism

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aldro61/mmit/pull/33#issuecomment-464801578, or mute the thread https://github.com/notifications/unsubscribe-auth/AA478pQHzD64xHvpwfEsZ3UjxB5MF28Bks5vOtX2gaJpZM4XvUjk .

[parismita]

Hi @tdhock , when I check using --as-cran, it gives a note

checking compiled code ... NOTE
File ‘mmit/libs/mmit.so’:
  Found ‘_ZSt4cout’, possibly from ‘std::cout’ (C++)
    Object: ‘piecewise_function.o’
File ‘mmit/libs/mmit.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.

I tried to change the std::cout to Rcpp:Rcout, but it throws error....what should be done in this case?

[tdhock]

well you probably should just delete the cout (usually C++ code should be silent if you are not debugging it)

about the register routines, see https://www.ironholds.org/registering-routines/

On Thu, Feb 21, 2019 at 3:17 AM parismita notifications@github.com wrote:

Hi, when I check using --as-cran, it gives a note

checking compiled code ... NOTE

File ‘mmit/libs/mmit.so’:

Found ‘_ZSt4cout’, possibly from ‘std::cout’ (C++)

Object: ‘piecewise_function.o’

File ‘mmit/libs/mmit.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.

I tried to change the std::cout to Rcpp:Rcout, but it throws error....what should be done in this case?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aldro61/mmit/pull/33#issuecomment-465943256, or mute the thread https://github.com/notifications/unsubscribe-auth/AA478jb7btpirntZjw2GOVcnUGQLT0tfks5vPnJEgaJpZM4XvUjk .

[parismita]

@tdhock I didnt understand the error that occured...is it because of the .c file?

[tdhock]

all builds fail because mvtnorm is not available for R 3.4.4 https://travis-ci.org/aldro61/mmit/jobs/508549725#L2560

that is a relatively old version of R

try upgrading the version of R used in .travis.yml

[aldro61]

@parismita Do you want to merge master into this branch to fix the R version problems?

Also, as discussed by email, the adaboost code got merged here by mistake. Since the mmitboost.R and mmitboost.predict.R files are not completely ready, we will delete them from here. However, we will keep the modified solver with example weights, since I already approved this code in another PR. Does this make sense?

[parismita]

@aldro61 Yeah, i'll pull from the master to update the travis part...

I'll delete the mmitboost part and make a new branch for adaboost for that

Thanks :)

[parismita]

@tdhock @aldro61 I updated it to make it cran submitable and deleted the adaboost R codes

[parismita]

@tdhock I didnt understand why the travis failed...what might have gone wrong?

[tdhock]

probably some network issue, in that case just click on re-run build (I already did)

[parismita]

@tdhock i think its because the branch is showing to be vignettes....it is supposed to be master

[tdhock]

https://stackoverflow.com/questions/39140800/the-command-eval-git-fetch-origin-refs-pull-7-merge-failed

[parismita]

@tdhock yeah...i updated the vignettes branch by merging from this branch...maybe the travis build was not completed then so its giving error.....but I am unable to revert the merge as I did commits after the merge...so currently I can rewrite the content of this branch to a new branch and have a pull req

[tdhock]

i would suggest avoiding re-writing the git history if at all possible.

the vignettes PR https://github.com/aldro61/mmit/pull/35 seems like it does NOT have the commits from this branch.

maybe try opening a new PR for this branch?

[parismita]

yeah...i'll be more carefull regarding pr, merges and git history next time...I'll try to open a new pr for this for now

[parismita]

@tdhock even this PR's build is clean...i dont understand how

[parismita]

ok