Memcheck2

[SermetPekin]

Pull Request Comments

Summary

This PR addresses an error related to package resolution caused by dependencies of dependencies. Specifically, some packages required by our package were dependent on other packages, which caused conflicts or issues during installation. Additionally, this PR includes the creation of a Dockerfile to streamline the setup and testing of the package in a consistent environment.

Changes Made

1. Updated Dockerfile:

   • Base Image: Switched to rocker/r-ver:4.3.0 for consistency and compatibility.
   • System Dependencies: Added necessary system dependencies to support package installations.
   • R Packages Installation:
     • Installed essential R packages (devtools, remotes, pak, covr, testthat).
     • Installed the latest version of Rcpp from GitHub to ensure compatibility with the latest features.
     • Explicitly installed Matrix to resolve dependencies required by fGarch.
     • Installed all other dependencies (fGarch, univariateML, kdensity, TreeDistData, TreeDist, phangorn).
   • Testing: Included a step to run tests using devtools::test() to ensure the package and its dependencies are functioning correctly.

2. Updated GitHub Actions Workflow:

   • Matrix Configuration: Ensured that tests are run on multiple operating systems and R versions, including R 3.4.0.
   • Dependencies Installation: Added steps to install devtools and remotes packages before using them to install other dependencies.
   • Run Tests: Added a step to run tests using devtools::test() after checking the package.
   • Code Coverage: Included code coverage reporting with covr::codecov().
   • Valgrind Checks:
     • Added specific requirements for Valgrind checks.
     • Enabled the verbose option in Valgrind to provide detailed output for memory checks.

How These Changes Address the Issue

• Dependency Resolution: The fGarch package was causing the resolution issue due to its dependency on Matrix (>= 1.5-0), which in turn required a newer version of R. By explicitly installing Matrix and other critical dependencies in the correct order, we prevent issues related to missing or conflicting dependencies.
• Compatibility: The updated workflow ensures compatibility with multiple R versions and operating systems, including older versions like R 3.4.0.
• Testing: Running tests during both the Docker build process and the CI workflow ensures that any issues with package functionality or compatibility are caught early.
• Code Coverage: Including code coverage reporting helps maintain high code quality and ensures that all parts of the package are tested.
• Valgrind Checks: The addition of specific requirements and enabling verbose mode in Valgrind helps to detect and diagnose any memory-related issues more effectively.
• Dockerfile: The addition of a Dockerfile allows for the creation of a consistent and reproducible environment for building, testing, and running the package. This is especially useful for ensuring that the package behaves correctly across different setups.

Next Steps

• Review: Please review the changes to ensure they meet the project’s standards.
• Testing: Verify that the package builds and installs correctly, and that all tests pass on the specified configurations.
• Dockerfile Testing: Validate that the Dockerfile builds successfully and that the package functions as expected within the Docker container.
• Valgrind Analysis: Review the Valgrind output for any memory-related issues.

Valgrind Analysis

The Valgrind analysis indicates that there are some "still reachable" memory blocks. This is not indicative of a memory leak but rather that some memory allocated during execution was still accessible at the end of the program. This is typically not a concern for short-lived processes or scripts, but we will continue to monitor memory usage to ensure there are no underlying issues.

Click to expand logs

``` ==15185== 238,800 bytes in 30 blocks are still reachable in loss record 2,476 of 2,616 ==15185== at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==15185== by 0x49F7514: GetNewPage (memory.c:998) ==15185== by 0x49F7902: Rf_allocSExp (memory.c:2443) ==15185== by 0x4A5E296: ReadBCLang (serialize.c:2144) ==15185== by 0x4A5E599: ReadBCConsts (serialize.c:2188) ==15185== by 0x4A5E599: ReadBC1 (serialize.c:2210) ==15185== by 0x4A5D61A: ReadBC (serialize.c:2221) ==15185== by 0x4A5D61A: ReadItem_Recursive (serialize.c:2052) ==15185== by 0x4A5C293: ReadItem_Iterative (serialize.c:1863) ==15185== by 0x4A5C293: ReadItem_Recursive (serialize.c:1963) ==15185== by 0x4A5CA89: ReadItem (serialize.c:2116) ==15185== by 0x4A5CA89: ReadItem_Iterative (serialize.c:1837) ==15185== by 0x4A5CA89: ReadItem_Recursive (serialize.c:1963) ==15185== by 0x4A5D950: ReadItem (serialize.c:2116) ==15185== by 0x4A5D950: ReadItem_Recursive (serialize.c:2103) ==15185== by 0x4A5CE3C: ReadItem (serialize.c:2116) ==15185== by 0x4A5CE3C: ReadItem_Recursive (serialize.c:2047) ==15185== by 0x4A5CA89: ReadItem (serialize.c:2116) ==15185== by 0x4A5CA89: ReadItem_Iterative (serialize.c:1837) ==15185== by 0x4A5CA89: ReadItem_Recursive (serialize.c:1963) ==15185== by 0x4A5D950: ReadItem (serialize.c:2116) ==15185== by 0x4A5D950: ReadItem_Recursive (serialize.c:2103) ==15185== ==15185== 238,800 bytes in 30 blocks are still reachable in loss record 2,477 of 2,616 ==15185== at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==15185== by 0x49F7514: GetNewPage (memory.c:998) ==15185== by 0x49F8FBB: Rf_allocVector3 (memory.c:2862) ==15185== by 0x49673D3: Rf_allocVector (Rinlinedfuns.h:595) ==15185== by 0x49673D3: duplicate1 (duplicate.c:352) ==15185== by 0x4966E21: duplicate_child (duplicate.c:206) ==15185== by 0x4966E21: duplicate_child (duplicate.c:204) ==15185== by 0x4966E21: duplicate_list (duplicate.c:268) ==15185== by 0x496768F: duplicate1 (duplicate.c:312) ==15185== by 0x496707E: duplicate1 (duplicate.c:352) ==15185== by 0x4966E21: duplicate_child (duplicate.c:206) ==15185== by 0x4966E21: duplicate_child (duplicate.c:204) ==15185== by 0x4966E21: duplicate_list (duplicate.c:268) ==15185== by 0x496768F: duplicate1 (duplicate.c:312) ==15185== by 0x49675F5: duplicate1 (duplicate.c:305) ==15185== by 0x49679DD: Rf_duplicate (duplicate.c:139) ==15185== by 0x49DE794: R_compute_identical (identical.c:126) ==15185== ==15185== 238,800 bytes in 30 blocks are still reachable in loss record 2,478 of 2,616 ==15185== at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==15185== by 0x49F7514: GetNewPage (memory.c:998) ==15185== by 0x49F83CE: CONS_NR (memory.c:2519) ==15185== by 0x499D697: bcEval_loop (eval.c:8213) ==15185== by 0x49B589C: bcEval (eval.c:7524) ==15185== by 0x49B589C: bcEval (eval.c:7509) ==15185== by 0x49B5C0A: Rf_eval (eval.c:1167) ==15185== by 0x49B7DDE: R_execClosure (eval.c:2398) ==15185== by 0x49B8BC6: applyClosure_core (eval.c:2311) ==15185== by 0x49BCCCC: Rf_applyClosure (eval.c:2333) ==15185== by 0x49BCCCC: do_recall (eval.c:4049) ==15185== by 0x49FC6E6: do_internal (names.c:1409) ==15185== by 0x499A71E: bcEval_loop (eval.c:8161) ==15185== by 0x49B589C: bcEval (eval.c:7524) ==15185== by 0x49B589C: bcEval (eval.c:7509) ==15185== by 0x49B5C0A: Rf_eval (eval.c:1167) ==15185== by 0x49B7DDE: R_execClosure (eval.c:2398) ==15185== ==15185== 238,800 bytes in 30 blocks are still reachable in loss record 2,479 of 2,616 ==15185== at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==15185== by 0x49F7514: GetNewPage (memory.c:998) ==15185== by 0x49F7902: Rf_allocSExp (memory.c:2443) ==15185== by 0x4A5C9CE: ReadItem_Iterative (serialize.c:1821) ==15185== by 0x4A5C9CE: ReadItem_Recursive (serialize.c:1963) ==15185== by 0x4A5CA89: ReadItem (serialize.c:2116) ==15185== by 0x4A5CA89: ReadItem_Iterative (serialize.c:1837) ==15185== by 0x4A5CA89: ReadItem_Recursive (serialize.c:1963) ==15185== by 0x4A5E543: ReadItem (serialize.c:2116) ==15185== by 0x4A5E543: ReadBCConsts (serialize.c:2193) ==15185== by 0x4A5E543: ReadBC1 (serialize.c:2210) ==15185== by 0x4A5D61A: ReadBC (serialize.c:2221) ==15185== by 0x4A5D61A: ReadItem_Recursive (serialize.c:2052) ==15185== by 0x4A5C293: ReadItem_Iterative (serialize.c:1863) ==15185== by 0x4A5C293: ReadItem_Recursive (serialize.c:1963) ==15185== by 0x4A5CE3C: ReadItem (serialize.c:2116) ==15185== by 0x4A5CE3C: ReadItem_Recursive (serialize.c:2047) ==15185== by 0x4A5CE3C: ReadItem (serialize.c:2116) ==15185== by 0x4A5CE3C: ReadItem_Recursive (serialize.c:2047) ==15185== by 0x4A5E95F: ReadItem (serialize.c:2116) ==15185== by 0x4A5E95F: R_Unserialize (serialize.c:2273) ==15185== by 0x4A5FE7A: R_unserialize (serialize.c:2991) ==15185== ==15185== 246,760 bytes in 31 blocks are still reachable in loss record 2,480 of 2,616 ==15185== at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==15185== by 0x49F7514: GetNewPage (memory.c:998) ==15185== by 0x49F7902: Rf_allocSExp (memory.c:2443) ==15185== by 0x4A5E296: ReadBCLang (serialize.c:2144) ==15185== by 0x4A5E320: ReadBCLang (serialize.c:2152) ==15185== by 0x4A5E343: ReadBCLang (serialize.c:2154) ==15185== by 0x4A5E343: ReadBCLang (serialize.c:2154) ==15185== by 0x4A5E320: ReadBCLang (serialize.c:2152) ==15185== by 0x4A5E343: ReadBCLang (serialize.c:2154) ==15185== by 0x4A5E343: ReadBCLang (serialize.c:2154) ==15185== by 0x4A5E320: ReadBCLang (serialize.c:2152) ==15185== by 0x4A5E343: ReadBCLang (serialize.c:2154) ==15185== ==15185== 246,760 bytes in 31 blocks are still reachable in loss record 2,481 of 2,616 ==15185== at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==15185== by 0x49F7514: GetNewPage (memory.c:998) ==15185== by 0x49F83CE: CONS_NR (memory.c:2519) ==15185== by 0x49A0931: bcEval_loop (eval.c:7994) ==15185== by 0x49B589C: bcEval (eval.c:7524) ==15185== by 0x49B589C: bcEval (eval.c:7509) ==15185== by 0x49B5C0A: Rf_eval (eval.c:1167) ==15185== by 0x49B65EC: forcePromise (eval.c:976) ==15185== by 0x49B65EC: forcePromise (eval.c:954) ==15185== by 0x49B5F37: Rf_eval (eval.c:1198) ==15185== by 0x49AD7C2: bcEval_loop (eval.c:8035) ==15185== by 0x49B589C: bcEval (eval.c:7524) ==15185== by 0x49B589C: bcEval (eval.c:7509) ==15185== by 0x49B5C0A: Rf_eval (eval.c:1167) ==15185== by 0x49B7DDE: R_execClosure (eval.c:2398) ==15185== ==15185== 262,680 bytes in 33 blocks are still reachable in loss record 2,482 of 2,616 ==15185== at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==15185== by 0x49F7514: GetNewPage (memory.c:998) ==15185== by 0x49F7902: Rf_allocSExp (memory.c:2443) ==15185== by 0x4A5E296: ReadBCLang (serialize.c:2144) ==15185== by 0x4A5E343: ReadBCLang (serialize.c:2154) ==15185== by 0x4A5E320: ReadBCLang (serialize.c:2152) ==15185== by 0x4A5E343: ReadBCLang (serialize.c:2154) ==15185== by 0x4A5

[ms609]

I have now revised the workflows to address the underpinning issues (compilation of TreeTools and TreeDist under different Rcpp versions), which addresses the issues motivating this PR. Thanks for looking into this and pointing me in the right direction.