tmp directory model compilation (#51)

* Initial commit for writing temp model fix

* Use winslash in normalizePath to remove need to grep for windows-specific path slashes.

* Define source file on which the hash is determined. The original file is still used for model compilation.

* Fix temp directory bug to ensure source file is hashed before model compilation of copied model file to temp.

* Don't show warning for creating test directory for running tests in same session.

* Run styler

* Hash the original source file into the right location during the init so we always have a baseline on the source model version.

* run styler

* Rebuild package documentation and fix notes following check. 1. Include mention of LICENSE file in DESCTIPTION and 2. add paper directory to .Rbuildignore as non-standard R package directory

* Update man/Model-class.Rd

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update R/MCSim_model.R

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Document package after fixing typo

* Remove unused file.copied variable when copying source file to model file in temp folder

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: tjzurlin

.Rbuildignore

@@ -3,6 +3,7 @@
 ^\.covrignore$
 ^\.github$
 ^.*\.Rproj$
+^paper$
 ^\_pkgdown.yml$
 .clang-format
 README.md

DESCRIPTION

@@ -15,11 +15,11 @@ Imports:
   methods,
   tools
 URL: https://CRAN.R-project.org/package=MCSimMod, https://github.com/USEPA/MCSimMod
-License: GPL-3
+License: GPL-3 + file LICENSE
 Encoding: UTF-8
 LazyData: true
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.3.2
+RoxygenNote: 7.3.3
 Suggests:
   knitr,
   rmarkdown,

R/MCSim_model.R

@@ -33,9 +33,10 @@ Model <- setRefClass("Model",
     #' @field paths List of character strings that are names of files associated with the model.
     #' @field writeTemp Boolean specifying whether to write model files to a temporary directory. If value is TRUE, model files will be written to a temporary directory; if value is FALSE, model files will be written to the same directory that contains the model specification file.
     #' @field verboseOutput Boolean specifying whether to write translator messages to standard output. If value is TRUE, messages will be written to standard output; if value is FALSE, messages will be written to files in a temporary directory.
+    #' @field recompiled Boolean specifying if the model has been recompiled due to change in source file
     mName = "character", mString = "character", initParms = "function",
     initStates = "function", Outputs = "ANY", parms = "numeric", Y0 = "numeric",
-    paths = "list", writeTemp = "logical", verboseOutput = "logical"
+    paths = "list", writeTemp = "logical", verboseOutput = "logical", recompiled = "logical"
   ),
   methods = list(
     initialize = function(...) {
@@ -53,20 +54,25 @@ Model <- setRefClass("Model",
         }
         file <- tempfile(pattern = "mcsimmod_", fileext = ".model")
         writeLines(mString, file)
+        source_file <- normalizePath(file, winslash = "/")
+        file <- source_file
       } else {
         if (writeTemp == TRUE) {
-          source_file <- normalizePath(paste0(mName, ".model"))
+          source_file <- normalizePath(paste0(mName, ".model"), winslash = "/")
           temp_directory <- tempdir()
           file <- file.path(temp_directory, basename(source_file))
-          file_copied <- file.copy(from = source_file, to = file)
+          file.copy(from = source_file, to = file)
+          file <- normalizePath(file, winslash = "/")
         } else {
-          file <- normalizePath(paste0(mName, ".model"))
+          source_file <- normalizePath(paste0(mName, ".model"), winslash = "/")
+          file <- source_file
         }
       }
       mList <- .fixPath(file)
+      sList <- .fixPath(source_file)
       mName <<- mList$mName
       mPath <- mList$mPath
-
+      sPath <- sList$mPath
 
       paths <<- list(
         dll_name = paste0(mName, "_model"),
@@ -75,17 +81,26 @@ Model <- setRefClass("Model",
         dll_file = file.path(mPath, paste0(mName, "_model", .Platform$dynlib.ext)),
         inits_file = file.path(mPath, paste0(mName, "_model_inits.R")),
         model_file = file.path(mPath, paste0(mName, ".model")),
+        source_file = file.path(sPath, paste0(mName, ".model")),
         hash_file = file.path(mPath, paste0(mName, "_model.md5"))
       )
+
+      # Calculate and save initial hash during initialization
+      # This allows loadModel to immediately check if source has changed
+      if (!file.exists(paths$hash_file)) {
+        initial_hash <- as.character(tools::md5sum(paths$source_file))
+        write(initial_hash, file = paths$hash_file)
+      }
     },
     loadModel = function(force = FALSE) {
       "Translate (if necessary) the model specification text to C, compile (if necessary) the resulting C file to create a dynamic link library (DLL) file (on Windows) or a shared object (SO) file (on Unix), and then load all essential information about the Model object into memory (for use in the current R session)."
       hash_exists <- file.exists(paths$hash_file)
       if (hash_exists) {
-        hash_has_changed <- .fileHasChanged(paths$model_file, paths$hash_file)
+        hash_has_changed <- .fileHasChanged(paths$source_file, paths$hash_file)
       } else {
         hash_has_changed <- TRUE
       }
+      recompiled <<- FALSE
 
       # Conditions for compiling a model:
       # 1. The DLL (on Windows) or SO (on Unix) associated with the model
@@ -98,7 +113,12 @@ Model <- setRefClass("Model",
       #    specification file has been changed since the last translation and
       #    compiling.
       if (!file.exists(paths$dll_file) | (force) | (!hash_exists) | (hash_exists & hash_has_changed)) {
-        compileModel(paths$model_file, paths$c_file, paths$dll_name, paths$dll_file, hash_file = paths$hash_file, verbose_output = verboseOutput)
+        # When using writeTemp=TRUE, ensure the model file is updated from source before compilation
+        if (writeTemp & (paths$source_file != paths$model_file)) {
+          file.copy(from = paths$source_file, to = paths$model_file, overwrite = TRUE)
+        }
+        compileModel(paths$model_file, paths$c_file, paths$dll_name, paths$dll_file, source_file = paths$source_file, hash_file = paths$hash_file, verbose_output = verboseOutput)
+        recompiled <<- TRUE
       }
 
       # Load the compiled model (DLL).

R/compileModel.R

@@ -8,13 +8,14 @@
 #' @param c_file Name of a C source code file to be created by compiling the MCSim model specification file.
 #' @param dll_name Name of a DLL or SO file without the extension (".dll" or ".so").
 #' @param dll_file Name of the same DLL or SO file with the appropriate extension (".dll" or ".so").
-#' @param hash_file Name of a file containing a hash key for determining if `model_file` has changed since the previous translation and compilation.
+#' @param source_file Name of the original source file to use for hash calculation. Defaults to \code{model_file} for backward compatibility. When \code{writeTemp=TRUE} in \code{createModel()}, this should be set to the original source file path to ensure hash tracking works correctly when the source file is separate from the compiled model file.
+#' @param hash_file Name of a file containing a hash key for determining if `source_file` has changed since the previous translation and compilation.
 #' @param verbose_output Boolean specifying whether to write translator messages to standard output. If value is TRUE, messages will be written to standard output; if value is FALSE, messages will be written to files in a temporary directory.
 #' @returns No return value. Creates files and saves them in locations specified by function arguments.
 #' @import tools
 #' @useDynLib MCSimMod, .registration=TRUE
 #' @export
-compileModel <- function(model_file, c_file, dll_name, dll_file, hash_file = NULL, verbose_output = FALSE) {
+compileModel <- function(model_file, c_file, dll_name, dll_file, source_file = model_file, hash_file = NULL, verbose_output = FALSE) {
   # Unload DLL if it has been loaded.
   mList <- .fixPath(model_file)
   model_name <- mList$mName
@@ -120,7 +121,7 @@ compileModel <- function(model_file, c_file, dll_name, dll_file, hash_file = NUL
     "event",
     "root"
   )
-  for (idx in seq(length(item_to_replace))) {
+  for (idx in seq_along(item_to_replace)) {
     lines <- gsub(
       paste0("\\b", item_to_replace[idx], "\\b"),
       paste0(item_to_replace[idx], "_", model_name),
@@ -151,7 +152,7 @@ compileModel <- function(model_file, c_file, dll_name, dll_file, hash_file = NUL
     "initStates",
     "initState"
   )
-  for (idx in seq(length(item_to_replace))) {
+  for (idx in seq_along(item_to_replace)) {
     lines <- gsub(
       paste0("\\b", item_to_replace[idx], "\\b"),
       paste0(item_to_replace[idx], "_", model_name),
@@ -181,10 +182,10 @@ compileModel <- function(model_file, c_file, dll_name, dll_file, hash_file = NUL
     normalizePath(out_file), ".\n"
   )
 
-  # If hash file name was provided, create a hash (md5 sum) for the model file
+  # If hash file name was provided, create a hash (md5 sum) for the source file
   # and print a message about its location.
   if (!is.null(hash_file)) {
-    file_hash <- as.character(md5sum(model_file))
+    file_hash <- as.character(tools::md5sum(source_file))
     write(file_hash, file = hash_file)
     message(
       "Hash created and saved in the file ", normalizePath(hash_file),

R/fileHasChanged.R

@@ -1,11 +1,11 @@
 #-----------------
 # compareHash
 #----------------
-# Private function to determine if the .model file has changed
+# Private function to determine if the source .model file has changed
 
-.fileHasChanged <- function(model_file, hash_file) {
+.fileHasChanged <- function(source_file, hash_file) {
   # Calculate hash for current model file
-  current_hash <- as.character(md5sum(model_file))
+  current_hash <- tools::md5sum(source_file)
 
   # Read saved hash
   saved_hash <- readLines(hash_file, n = 1)

R/fixPath.R

@@ -7,9 +7,6 @@
 .fixPath <- function(file) {
   new.mName <- strsplit(basename(file), "[.]")[[1]][1]
   new.mPath <- dirname(file)
-  if (.Platform$OS.type == "windows") {
-    new.mPath <- gsub("\\\\", "/", utils::shortPathName(new.mPath))
-  }
 
   has_space <- grepl(" ", new.mPath)
   if (has_space == T) {

man/Model-class.Rd

@@ -22,3 +22,50 @@ testthat::test_that("test_compareHash", {
 
   model$cleanup()
 })
+
+testthat::test_that("test_tmp_compileModel", {
+  # Test writeTemp=TRUE functionality with proper source/compilation file separation
+  # Create a proper source file in a permanent location (not temp)
+  source_dir <- file.path(tempdir(), "sourceDir")
+  dir.create(source_dir, showWarnings = FALSE)
+  mName <- file.path(source_dir, "test_model")
+  mString <- readLines(file.path(testthat::test_path(), "data", "exponential.model"))
+  writeLines(mString, paste0(mName, ".model"))
+
+  # Verify source file exists
+  testthat::expect_true(file.exists(paste0(mName, ".model")))
+
+  # Create model with writeTemp=TRUE - this should use the source file
+  # and create compilation files in temp directory
+  model <- createModel(mName, writeTemp = TRUE)
+
+  # Verify paths are properly separated
+  testthat::expect_true(model$paths$source_file != model$paths$model_file)
+  testthat::expect_true(file.exists(model$paths$source_file))
+  testthat::expect_true(file.exists(model$paths$model_file))
+
+  # First load - should compile
+  model$loadModel()
+  testthat::expect_true(file.exists(model$paths$hash_file)) # Check if hash was created
+  testthat::expect_true(model$recompiled) # Should be TRUE on first compile
+
+  # Second load - should NOT recompile (no changes)
+  model$loadModel()
+  testthat::expect_false(.fileHasChanged(model$paths$source_file, model$paths$hash_file))
+  testthat::expect_false(model$recompiled) # Should be FALSE, no recompile needed
+
+  # Edit source file (what users actually edit)
+  write("# File is edited", file = model$paths$source_file, append = TRUE, sep = "\n")
+  testthat::expect_true(.fileHasChanged(model$paths$source_file, model$paths$hash_file))
+
+  # Third load after edit - should recompile
+  model$loadModel()
+  testthat::expect_true(model$recompiled) # Should be TRUE, source changed
+
+  # Fourth load - should NOT recompile (our bug fix test!)
+  model$loadModel()
+  testthat::expect_false(.fileHasChanged(model$paths$source_file, model$paths$hash_file))
+  testthat::expect_false(model$recompiled) # Should be FALSE, hash should now match
+
+  model$cleanup()
+})

man/compileModel.Rd

@@ -45,7 +45,7 @@ testthat::test_that("Model$absoluteModel", {
   # Use absolute path of temp directory,
   # Test to make sure changing the file returns a changed path
 
-  dir.create(file.path(tempdir(), "testDir"))
+  dir.create(file.path(tempdir(), "testDir"), showWarnings = FALSE)
   mName <- tempfile(pattern = "mcsimmod_", tmpdir = file.path(tempdir(), "testDir"))
   mString <- readLines(file.path(testthat::test_path(), "data", "exponential.model"))
   writeLines(mString, paste0(mName, ".model"))