Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

startSocsimWithFile() crashes when supfile contains more than a basename #20

Open
ohexel opened this issue Nov 26, 2024 · 4 comments
Open

Comments

@ohexel
Copy link

ohexel commented Nov 26, 2024

If supfile contains more than the basename, i.e. some_directory/my_sup_file.sup instead of my_sup_file.sup, the process will crash inside startSocsimWithFile().

I'd guess the problem is in line 277 or 275 of events.cpp, based on what gets written to stdout before the crash.

The documentation is wrong to state that every sup and rate file should be referred to relative to folder. Currently, sup and rate files must be directly under folder for everything to work.

dir.create("tmp/tmp", recursive = TRUE)
tmpdir <- file.path(getwd(), "tmp/tmp")
dir.exists(tmpdir)
supname <- create_sup_file(simfolder = tmpdir)
file.exists(file.path(tmpdir, supname))
file.exists(file.path(getwd(), "tmp/tmp", supname))
identical(file.path(tmpdir, supname),
          file.path(getwd(), "tmp/tmp", supname))
# This works
socsim(folder = tmpdir,
       supfile = supname)
# This does not work
socsim(folder = file.path(getwd(), "tmp"),
       supfile = file.path("tmp", supname))

The behavior happens whether the base directory is inside the working directory or not.

dir.create("U:/tmp/tmp", recursive = TRUE)
tmpdir <- "U:/tmp/tmp"
dir.exists(tmpdir)
supname <- create_sup_file(simfolder = tmpdir)
file.exists(file.path(tmpdir, supname))
file.exists(file.path("U:/tmp", "tmp", supname))
identical(file.path(tmpdir, supname),
          file.path("U:/tmp", "tmp", supname))
# This works
socsim(folder = tmpdir,
       supfile = supname)
# This does not work
socsim(folder = "U:/tmp",
       supfile = file.path("tmp", supname))
@ohexel
Copy link
Author

ohexel commented Nov 29, 2024

The error is in create_output_fn_dir(). Socsim does not check whether the input rate file exists. Same for the output file (in this case, the problem is probably whether it can be created or not). I think this is because neither main1() nor create_output_fn_dir() do any sanity or consistency checks on the naming of input variables.

I question why create_output_fn_dir() should exist as a separate function at all. It takes no arguments but assumes the existence of a whole bunch of variables. It is only called in one place. Seems more clear and safer to me to integrate it into main1().

@ohexel
Copy link
Author

ohexel commented Nov 29, 2024

To check, check whether opening the rate file or the destination file succeeded (below). It doesn't. The error message propagates through to R (which isn't true for fprintf(stderr, "my error") or printf("my message")?!).

FILE *source = fopen(rate_file_name, "rb")
if (source == NULL) {
    perror("Error opening source (rate) file.");
    //if you uncomment the below, main1() will continue (!)
    // to run and crash when trying to create the log file.
    //return 1; 
}

@tomthe
Copy link
Member

tomthe commented Nov 29, 2024

I question why create_output_fn_dir() should exist as a separate function at all.

I think it should, because it makes the code more readable. main1() is already too long. create_output_fn_dir() does one thing that you can compartementalize and put it into another place. It uses global state and assumes the existence of some variables, which is not how we would write this in 2024... but that was written in the 80ies or 90ies and if you want to improve the codebase, this is unfortunately not the worst thing :)

I don't hate Rprintf() - why do you think I do? Because there are these Rcpp::Rcout << "..."; statements? Do you hate them? Why?

I don't fully understand what causes the error yet. Is it the block where it copies the .sup-file into the subfolder (load.cpp line 366 and further)?

@ohexel
Copy link
Author

ohexel commented Nov 29, 2024

Rprintf(): seems less cumbersome to me, but the great machine spirit informed me that Rcpp::Rcout is preferable in Rcpp contexts.

Cause: yes. It cannot find the rate file or destination file and the copy step fails.

This seems to be because the Rcpp process happens in a different directory than the R process. Still investigating this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants