Added new controls to separationDetection()

[gthopkins]

We adjusted the algorithm used to perform separation detection, hoping that this might eliminate our dependence on the slam package and allowing corncob to be compatible with early versions of R. This is in response to issue #177. We hope this will be a fix, but this may not resolve the error.

I also renamed a vector called 'select', because both the MASS and dplyr packages have functions called 'select'. These are in constant competition and frequently cause coding mishaps, so no need for another object with the same name. This is a matter of preference.

[gthopkins]

@svteichman this PR passed all existing tests, however it failed the pkgdown portion of the GitHub check. I am unfamilair with pkgdown, however it seems that this is only being used to store the corncob logo. If we can ignore this issue, I think this PR is good to go and might resolve issue #177.

My previous message does not seem to be posted, so I will explain my reasoning here. I think corncob depends on slam as follows: The separationDetection() is a wrapper for detectseparation::detect_separation(), which depends on the ROI package. Finally, the ROI package depends on slam in some way that is not obvious to me. I think we can avoid the dependency on slam by switching our seperation detection to use a different algorithm. This is just a hunch, but it is worth an effort to see if we can keep corncob compatible with earlier versions of R.

[svteichman]

@gthopkins I just dealt with this packagedown issue in PR #179. Try pulling these recent changes into your fork (whichever branch this PR is from) and then let the checks run again and see if this issue goes away. This happened last month to radEmu as well - the issue was that the document "logo.png" was in the "docs" folder, and pkgdown wants there to be no documents in this folder except for the ones it creates. I don't know why this issue is just coming up now, but it should be unrelated to the changes you made. That being said, I'd like to wait until all checks pass to merge this PR.

[svteichman]

@gthopkins this fix looks good to me, once it is merged, let's follow-up with the person who posted the issue and see if this PR fixes their problem, and if so, we can close that issue.

[gthopkins]

@svteichman Thank you for clarifying how to fix the packagedown issue. The checks now complete without error and I merged the PR. I will follow-up now to check if this resolves the issue.

[adw96]

Hi @gthopkins ! Would you be able to please give me a quick rundown on what happened here? Did slam (?) get deprecated? I don’t quite understand the issue nor the fix. Any ideas why this user (and not other users) have this issue?

Thanks for fixing and I am grateful for a mini post-op report so we have it as a reference.

[gthopkins]

@adw96 The main source of the error is that the R version is not up-to-date, however I believe the slam package makes use of a group generic that is only defined for R(>=4.3.0), as far as I can tell. Here is a sketch of the issue:

1. separationDetection() is a wrapper for detectseparation::detect_separation()
2. By default, detect_separation() by default uses the ROI package for optimization
3. the ROI package depends on the slam package
4. As seen in this GitHub mirror, the slam package has the following code in its namespace:

if(getRversion() >= "4.3.0") {
   S3method("matrixOps", "simple_triplet_matrix")
   S3method("chooseOpsMethod", "simple_triplet_matrix")
}

5. The user has R version 4.1.1, so the S3 method is not written here
6. I do not understand advanced R object-oriented underpinnings to figure this out with certainty, however it seems that perhaps slam::matrixOps.simple_triplet_matrix() or another function refers to the "matrixOps" group-generic, which was only created in R version 4.3.0.

The matrix multiply operator %*% is now an S3 generic, belonging to new group generic matrixOps.

Perhaps you have more insight as to what the exact incompatibility is between R versions here, but I do expect the issue arises from this recent update to the slam package.

In terms of my solution, I am trying to eliminate our dependence on the slam package. To do this, I intervene in Step 2 of the issue above and use the control argument in detectseparation::detect_separation() to use the lpSolveAPI package for optimization instead of the ROI package. I am still waiting to hear from the original user if this indeed resolves the issue.