address reviewer comments

[tdhock]

in this PR I hope to address all the comments from the JSS reviews.

[tdhock]

valgrind on rhub says

> test_check("PeakSegDisk", filter=suite)
Loading required package: PeakSegDisk
data_i+data_count=2 before writing down cost with 1 pieces.
seek_element(2)
seek_element(2)
==3371== Syscall param write(buf) points to uninitialised byte(s)
==3371==    at 0x4D14471: write (write.c:26)
==3371==    by 0x784AD65: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==3371==    by 0x7883AC1: std::basic_filebuf<char, std::char_traits<char> >::_M_convert_to_external(char*, long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==3371==    by 0x7883E83: std::basic_filebuf<char, std::char_traits<char> >::overflow(int) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==3371==    by 0x7883C0D: std::basic_filebuf<char, std::char_traits<char> >::_M_terminate_output() (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==3371==    by 0x7883D0A: std::basic_filebuf<char, std::char_traits<char> >::_M_seek(long, std::_Ios_Seekdir, __mbstate_t) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==3371==    by 0x7884164: std::basic_filebuf<char, std::char_traits<char> >::seekoff(long, std::_Ios_Seekdir, std::_Ios_Openmode) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==3371==    by 0x78A4F55: std::ostream::seekp(long, std::_Ios_Seekdir) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==3371==    by 0xD0F9A4F: seek_element (PeakSegFPOPLog.cpp:87)
==3371==    by 0xD0F9A4F: write (PeakSegFPOPLog.cpp:127)
==3371==    by 0xD0F9A4F: PeakSegFPOP_disk(char*, char*) (PeakSegFPOPLog.cpp:383)
==3371==    by 0xD0FE666: PeakSegFPOP_interface(char**, char**) (interface.cpp:14)
==3371==    by 0x4937C9B: ??? (in /usr/lib/R/lib/libR.so)
==3371==    by 0x497C7EF: Rf_eval (in /usr/lib/R/lib/libR.so)
==3371==  Address 0xa00f688 is 56 bytes inside a block of size 8,192 alloc'd
==3371==    at 0x483650F: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==3371==    by 0x7883987: std::basic_filebuf<char, std::char_traits<char> >::_M_allocate_internal_buffer() (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==3371==    by 0x7887821: std::basic_filebuf<char, std::char_traits<char> >::open(char const*, std::_Ios_Openmode) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==3371==    by 0xD0F94E2: open (fstream:1076)
==3371==    by 0xD0F94E2: init (PeakSegFPOPLog.cpp:71)
==3371==    by 0xD0F94E2: PeakSegFPOP_disk(char*, char*) (PeakSegFPOPLog.cpp:237)
==3371==    by 0xD0FE666: PeakSegFPOP_interface(char**, char**) (interface.cpp:14)
==3371==    by 0x4937C9B: ??? (in /usr/lib/R/lib/libR.so)
==3371==    by 0x497C7EF: Rf_eval (in /usr/lib/R/lib/libR.so)
==3371==    by 0x4980987: ??? (in /usr/lib/R/lib/libR.so)
==3371==    by 0x497C5A8: Rf_eval (in /usr/lib/R/lib/libR.so)
==3371==    by 0x497F755: ??? (in /usr/lib/R/lib/libR.so)
==3371==    by 0x497C5A8: Rf_eval (in /usr/lib/R/lib/libR.so)
==3371==    by 0x498184E: ??? (in /usr/lib/R/lib/libR.so)
==3371== 
data_i=1 before writing up cost with 1 pieces.
seek_element(1)
seek_element(1)
data_i+data_count=3 before writing down cost with 1 pieces.

[tdhock]

new CRAN says

==3342== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==3342== Command: /data/blackswan/ripley/R/R-devel-vg/bin/exec/R --vanilla
==3342== 

R Under development (unstable) (2019-07-23 r76870) -- "Unsuffered Consequences"
Copyright (C) 2019 The R Foundation for Statistical Computing
Platform: x86_64-pc-linux-gnu (64-bit)

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.

  Natural language support but running in an English locale

R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.

Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.

> pkgname <- "PeakSegDisk"
> source(file.path(R.home("share"), "R", "examples-header.R"))
> options(warn = 1)
> library('PeakSegDisk')
> 
> base::assign(".oldSearch", base::search(), pos = 'CheckExEnv')
> base::assign(".old_wd", base::getwd(), pos = 'CheckExEnv')
> cleanEx()
> nameEx("PeakSegFPOP_disk")
> ### * PeakSegFPOP_disk
> 
> flush(stderr()); flush(stdout())
> 
> ### Name: PeakSegFPOP_disk
> ### Title: PeakSegFPOP on disk
> ### Aliases: PeakSegFPOP_disk
> 
> ### ** Examples
> 
> 
> 
> 
> 
> 
> library(PeakSegDisk)
> 
> 
> r <- function(chrom, chromStart, chromEnd, coverage){
+ 
+ 
+   data.frame(chrom, chromStart, chromEnd, coverage)
+ 
+ 
+ }
> 
> 
> four <- rbind(
+ 
+ 
+   r("chr1", 0, 10,  2),
+ 
+ 
+   r("chr1", 10, 20, 10),
+ 
+ 
+   r("chr1", 20, 30, 14),
+ 
+ 
+   r("chr1", 30, 40, 13))
> 
> 
> write.table(
+ 
+ 
+   four, tmp <- tempfile(),
+ 
+ 
+   sep="\t", row.names=FALSE, col.names=FALSE)
> 
> 
> names.list <- PeakSegFPOP_disk(tmp, "10.5")
==3342== Syscall param write(buf) points to uninitialised byte(s)
==3342==    at 0x53FFB15: write (in /usr/lib64/libpthread-2.29.so)
==3342==    by 0x72451E5: ??? (in /usr/lib64/libstdc++.so.6.0.26)
==3342==    by 0x7284CC4: std::basic_filebuf<char, std::char_traits<char> >::_M_convert_to_external(char*, long) (in /usr/lib64/libstdc++.so.6.0.26)
==3342==    by 0x728514B: std::basic_filebuf<char, std::char_traits<char> >::overflow(int) (in /usr/lib64/libstdc++.so.6.0.26)
==3342==    by 0x7284E55: std::basic_filebuf<char, std::char_traits<char> >::_M_terminate_output() (in /usr/lib64/libstdc++.so.6.0.26)
==3342==    by 0x7284F9E: std::basic_filebuf<char, std::char_traits<char> >::_M_seek(long, std::_Ios_Seekdir, __mbstate_t) (in /usr/lib64/libstdc++.so.6.0.26)
==3342==    by 0x7285455: std::basic_filebuf<char, std::char_traits<char> >::seekoff(long, std::_Ios_Seekdir, std::_Ios_Openmode) (in /usr/lib64/libstdc++.so.6.0.26)
==3342==    by 0x72AD1FE: std::ostream::seekp(long, std::_Ios_Seekdir) (in /usr/lib64/libstdc++.so.6.0.26)
==3342==    by 0x48C38C8: seek_element (packages/tests-vg/PeakSegDisk/src/PeakSegFPOPLog.cpp:91)
==3342==    by 0x48C38C8: write (packages/tests-vg/PeakSegDisk/src/PeakSegFPOPLog.cpp:131)
==3342==    by 0x48C38C8: PeakSegFPOP_disk(char*, char*) (packages/tests-vg/PeakSegDisk/src/PeakSegFPOPLog.cpp:405)
==3342==    by 0x48C8507: PeakSegFPOP_interface(char**, char**) (packages/tests-vg/PeakSegDisk/src/interface.cpp:11)
==3342==    by 0x498FFD: do_dotCode (svn/R-devel/src/main/dotcode.c:1746)
==3342==    by 0x4CCC8F: bcEval (svn/R-devel/src/main/eval.c:6775)
==3342==  Address 0x100919a8 is 56 bytes inside a block of size 8,192 alloc'd
==3342==    at 0x4839593: operator new[](unsigned long) (/builddir/build/BUILD/valgrind-3.15.0/coregrind/m_replacemalloc/vg_replace_malloc.c:433)
==3342==    by 0x7284B23: std::basic_filebuf<char, std::char_traits<char> >::_M_allocate_internal_buffer() (in /usr/lib64/libstdc++.so.6.0.26)
==3342==    by 0x7288E36: std::basic_filebuf<char, std::char_traits<char> >::open(char const*, std::_Ios_Openmode) (in /usr/lib64/libstdc++.so.6.0.26)
==3342==    by 0x48C31CC: open (/usr/include/c++/9/fstream:1180)
==3342==    by 0x48C31CC: init (packages/tests-vg/PeakSegDisk/src/PeakSegFPOPLog.cpp:73)
==3342==    by 0x48C31CC: PeakSegFPOP_disk(char*, char*) (packages/tests-vg/PeakSegDisk/src/PeakSegFPOPLog.cpp:263)
==3342==    by 0x48C8507: PeakSegFPOP_interface(char**, char**) (packages/tests-vg/PeakSegDisk/src/interface.cpp:11)
==3342==    by 0x498FFD: do_dotCode (svn/R-devel/src/main/dotcode.c:1746)
==3342==    by 0x4CCC8F: bcEval (svn/R-devel/src/main/eval.c:6775)
==3342==    by 0x4E4DFF: Rf_eval (svn/R-devel/src/main/eval.c:620)
==3342==    by 0x4E694E: R_execClosure (svn/R-devel/src/main/eval.c:1780)
==3342==    by 0x4E7694: Rf_applyClosure (svn/R-devel/src/main/eval.c:1706)
==3342==    by 0x4E4EF4: Rf_eval (svn/R-devel/src/main/eval.c:743)
==3342==    by 0x4E908E: do_set (svn/R-devel/src/main/eval.c:2808)
==3342==  Uninitialised value was created by a heap allocation
==3342==    at 0x4838E86: operator new(unsigned long) (/builddir/build/BUILD/valgrind-3.15.0/coregrind/m_replacemalloc/vg_replace_malloc.c:344)
==3342==    by 0x48C3431: allocate (/usr/include/c++/9/ext/new_allocator.h:114)
==3342==    by 0x48C3431: allocate (/usr/include/c++/9/bits/alloc_traits.h:444)
==3342==    by 0x48C3431: _M_get_node (/usr/include/c++/9/bits/stl_list.h:438)
==3342==    by 0x48C3431: _M_create_node<double, int, double, double&, double&, int, bool> (/usr/include/c++/9/bits/stl_list.h:630)
==3342==    by 0x48C3431: _M_insert<double, int, double, double&, double&, int, bool> (/usr/include/c++/9/bits/stl_list.h:1907)
==3342==    by 0x48C3431: emplace_back<double, int, double, double&, double&, int, bool> (/usr/include/c++/9/bits/stl_list.h:1223)
==3342==    by 0x48C3431: PeakSegFPOP_disk(char*, char*) (packages/tests-vg/PeakSegDisk/src/PeakSegFPOPLog.cpp:284)
==3342==    by 0x48C8507: PeakSegFPOP_interface(char**, char**) (packages/tests-vg/PeakSegDisk/src/interface.cpp:11)
==3342==    by 0x498FFD: do_dotCode (svn/R-devel/src/main/dotcode.c:1746)
==3342==    by 0x4CCC8F: bcEval (svn/R-devel/src/main/eval.c:6775)
==3342==    by 0x4E4DFF: Rf_eval (svn/R-devel/src/main/eval.c:620)
==3342==    by 0x4E694E: R_execClosure (svn/R-devel/src/main/eval.c:1780)
==3342==    by 0x4E7694: Rf_applyClosure (svn/R-devel/src/main/eval.c:1706)
==3342==    by 0x4E4EF4: Rf_eval (svn/R-devel/src/main/eval.c:743)
==3342==    by 0x4E908E: do_set (svn/R-devel/src/main/eval.c:2808)
==3342==    by 0x4E5162: Rf_eval (svn/R-devel/src/main/eval.c:695)
==3342==    by 0x5137FC: Rf_ReplIteration (svn/R-devel/src/main/main.c:260)
==3342==    by 0x5137FC: Rf_ReplIteration (svn/R-devel/src/main/main.c:200)
==3342== 
> 
> 
> unlink(names.list$db)

[tdhock]

==1068== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==1068== Command: /home/Hornik/tmp/R-d-gcc-8/bin/exec/R --vanilla
==1068== 
R Under development (unstable) (2019-07-03 r76775) -- "Unsuffered Consequences"
Copyright (C) 2019 The R Foundation for Statistical Computing
Platform: x86_64-pc-linux-gnu (64-bit)

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.

  Natural language support but running in an English locale

R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.

Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.

> pkgname <- "PeakSegDisk"
> source(file.path(R.home("share"), "R", "examples-header.R"))
> options(warn = 1)
> library('PeakSegDisk')
> 
> base::assign(".oldSearch", base::search(), pos = 'CheckExEnv')
> base::assign(".old_wd", base::getwd(), pos = 'CheckExEnv')
> cleanEx()
> nameEx("PeakSegFPOP_disk")
> ### * PeakSegFPOP_disk
> 
> flush(stderr()); flush(stdout())
> 
> ### Name: PeakSegFPOP_disk
> ### Title: PeakSegFPOP on disk
> ### Aliases: PeakSegFPOP_disk
> 
> ### ** Examples
> 
> 
> 
> 
> 
> 
> library(PeakSegDisk)
> 
> 
> r <- function(chrom, chromStart, chromEnd, coverage){
+ 
+ 
+   data.frame(chrom, chromStart, chromEnd, coverage)
+ 
+ 
+ }
> 
> 
> four <- rbind(
+ 
+ 
+   r("chr1", 0, 10,  2),
+ 
+ 
+   r("chr1", 10, 20, 10),
+ 
+ 
+   r("chr1", 20, 30, 14),
+ 
+ 
+   r("chr1", 30, 40, 13))
> 
> 
> write.table(
+ 
+ 
+   four, tmp <- tempfile(),
+ 
+ 
+   sep="\t", row.names=FALSE, col.names=FALSE)
> 
> 
> names.list <- PeakSegFPOP_disk(tmp, "10.5")
==1068== Syscall param write(buf) points to uninitialised byte(s)
==1068==    at 0x4D7B471: write (/build/glibc-vjB4T1/glibc-2.28/nptl/../sysdeps/unix/sysv/linux/write.c:26)
==1068==    by 0x78C9D65: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==1068==    by 0x7902AC1: std::basic_filebuf<char, std::char_traits<char> >::_M_convert_to_external(char*, long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==1068==    by 0x7902E83: std::basic_filebuf<char, std::char_traits<char> >::overflow(int) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==1068==    by 0x7902C0D: std::basic_filebuf<char, std::char_traits<char> >::_M_terminate_output() (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==1068==    by 0x7902D0A: std::basic_filebuf<char, std::char_traits<char> >::_M_seek(long, std::_Ios_Seekdir, __mbstate_t) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==1068==    by 0x7903164: std::basic_filebuf<char, std::char_traits<char> >::seekoff(long, std::_Ios_Seekdir, std::_Ios_Openmode) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==1068==    by 0x7923F55: std::ostream::seekp(long, std::_Ios_Seekdir) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==1068==    by 0x19B8093C: seek_element (/home/Hornik/tmp/CRAN/PeakSegDisk.Rcheck/00_pkg_src/PeakSegDisk/src/PeakSegFPOPLog.cpp:86)
==1068==    by 0x19B8093C: write (/home/Hornik/tmp/CRAN/PeakSegDisk.Rcheck/00_pkg_src/PeakSegDisk/src/PeakSegFPOPLog.cpp:126)
==1068==    by 0x19B8093C: PeakSegFPOP_disk(char*, char*) (/home/Hornik/tmp/CRAN/PeakSegDisk.Rcheck/00_pkg_src/PeakSegDisk/src/PeakSegFPOPLog.cpp:378)
==1068==    by 0x19B854B6: PeakSegFPOP_interface(char**, char**) (/home/Hornik/tmp/CRAN/PeakSegDisk.Rcheck/00_pkg_src/PeakSegDisk/src/interface.cpp:14)
==1068==    by 0x49376DB: do_dotCode (/home/Hornik/src/R/src/main/dotcode.c:1746)
==1068==    by 0x496B54B: bcEval (/home/Hornik/src/R/src/main/eval.c:6775)
==1068==  Address 0x165db388 is 56 bytes inside a block of size 8,192 alloc'd
==1068==    at 0x483650F: operator new[](unsigned long) (coregrind/m_replacemalloc/vg_replace_malloc.c:423)
==1068==    by 0x7902987: std::basic_filebuf<char, std::char_traits<char> >::_M_allocate_internal_buffer() (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==1068==    by 0x7906821: std::basic_filebuf<char, std::char_traits<char> >::open(char const*, std::_Ios_Openmode) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==1068==    by 0x19B8040C: open (/usr/include/c++/8/fstream:1076)
==1068==    by 0x19B8040C: init (/home/Hornik/tmp/CRAN/PeakSegDisk.Rcheck/00_pkg_src/PeakSegDisk/src/PeakSegFPOPLog.cpp:71)
==1068==    by 0x19B8040C: PeakSegFPOP_disk(char*, char*) (/home/Hornik/tmp/CRAN/PeakSegDisk.Rcheck/00_pkg_src/PeakSegDisk/src/PeakSegFPOPLog.cpp:236)
==1068==    by 0x19B854B6: PeakSegFPOP_interface(char**, char**) (/home/Hornik/tmp/CRAN/PeakSegDisk.Rcheck/00_pkg_src/PeakSegDisk/src/interface.cpp:14)
==1068==    by 0x49376DB: do_dotCode (/home/Hornik/src/R/src/main/dotcode.c:1746)
==1068==    by 0x496B54B: bcEval (/home/Hornik/src/R/src/main/eval.c:6775)
==1068==    by 0x497C2DF: Rf_eval (/home/Hornik/src/R/src/main/eval.c:620)
==1068==    by 0x497E06E: R_execClosure (/home/Hornik/src/R/src/main/eval.c:1780)
==1068==    by 0x497EE9A: Rf_applyClosure (/home/Hornik/src/R/src/main/eval.c:1706)
==1068==    by 0x497C494: Rf_eval (/home/Hornik/src/R/src/main/eval.c:743)
==1068==    by 0x4980A26: do_set (/home/Hornik/src/R/src/main/eval.c:2808)
==1068==  Uninitialised value was created by a heap allocation
==1068==    at 0x4835DEF: operator new(unsigned long) (coregrind/m_replacemalloc/vg_replace_malloc.c:334)
==1068==    by 0x19B80665: allocate (/usr/include/c++/8/ext/new_allocator.h:111)
==1068==    by 0x19B80665: allocate (/usr/include/c++/8/bits/alloc_traits.h:436)
==1068==    by 0x19B80665: _M_get_node (/usr/include/c++/8/bits/stl_list.h:450)
==1068==    by 0x19B80665: _M_create_node<double, int, double, double&, double&, int, bool> (/usr/include/c++/8/bits/stl_list.h:642)
==1068==    by 0x19B80665: _M_insert<double, int, double, double&, double&, int, bool> (/usr/include/c++/8/bits/stl_list.h:1903)
==1068==    by 0x19B80665: emplace_back<double, int, double, double&, double&, int, bool> (/usr/include/c++/8/bits/stl_list.h:1235)
==1068==    by 0x19B80665: PeakSegFPOP_disk(char*, char*) (/home/Hornik/tmp/CRAN/PeakSegDisk.Rcheck/00_pkg_src/PeakSegDisk/src/PeakSegFPOPLog.cpp:256)
==1068==    by 0x19B854B6: PeakSegFPOP_interface(char**, char**) (/home/Hornik/tmp/CRAN/PeakSegDisk.Rcheck/00_pkg_src/PeakSegDisk/src/interface.cpp:14)
==1068==    by 0x49376DB: do_dotCode (/home/Hornik/src/R/src/main/dotcode.c:1746)
==1068==    by 0x496B54B: bcEval (/home/Hornik/src/R/src/main/eval.c:6775)
==1068==    by 0x497C2DF: Rf_eval (/home/Hornik/src/R/src/main/eval.c:620)
==1068==    by 0x497E06E: R_execClosure (/home/Hornik/src/R/src/main/eval.c:1780)
==1068==    by 0x497EE9A: Rf_applyClosure (/home/Hornik/src/R/src/main/eval.c:1706)
==1068==    by 0x497C494: Rf_eval (/home/Hornik/src/R/src/main/eval.c:743)
==1068==    by 0x4980A26: do_set (/home/Hornik/src/R/src/main/eval.c:2808)
==1068==    by 0x497C6EC: Rf_eval (/home/Hornik/src/R/src/main/eval.c:695)
==1068==    by 0x49ACE79: Rf_ReplIteration (/home/Hornik/src/R/src/main/main.c:260)
==1068==    by 0x49ACE79: Rf_ReplIteration (/home/Hornik/src/R/src/main/main.c:200)
==1068== 
> 
> 
> unlink(names.list$db)

[tdhock]

maybe has something to do with the fact that emplace_back does not use the same types as constructor?

==1068==    by 0x19B80665: emplace_back<double, int, double, double&, double&, int, bool> (/usr/include/c++/8/bits/stl_list.h:1235)

    down_cost.piece_list.emplace_back
      (1.0, -coverage, 0.0,
min_log_mean, max_log_mean, -1, false);

PoissonLossPieceLog::PoissonLossPieceLog
(double li, double lo, double co, double m, double M, int i, double prev){
  Linear = li;
  Log = lo;
  Constant = co;
  min_log_mean = m;
  max_log_mean = M;
  data_i = i;
  prev_log_mean = prev;
}

[tdhock]

https://r-hub.github.io/rhub/articles/local-debugging.html

[tdhock]

fixed via https://tdhock.github.io/blog/2019/docker-on-mac/