assorted fixes

[jeffreyhanson]

This PR provides several fixes. These include (1) fix compatibility issues with upcoming version of the Matrix package (version 1.4-2), (2) update package documentation to provide details for obtaining and installing the cplexAPI package since it has been archived on CRAN, (3) fix bug that caused the add_cbc_solver() to throw a segfault when solving a problem wherein the rij_matrix(x) has a zero amount for the last feature in the last planning unit (#247), and (4) update simulate_data(), simulate_cost() and simulate_species() functions to improve performance using the fields package.

When you get a chance, @ricschuster, could you please take a look? There's a lot of changes throughout the package, most of which involve replacing as(x, "MATRIX_CLASS") with a wrapper as_Matrix(x, "MATRIX_CLASS) to ensure compatibility with present and upcomming version of the Matrix package. I'm hoping that we can resubmit the package and include cplexAPI as an optional dependency as long as we say in the DESCRIPTION/documentation where/how to install it. I've posted a question on the ROpenSci slack and will report back once I hear if anyone's been able to pull this off.

[jeffreyhanson]

@ricschuster, also, if you could please double check that the package works on your system with the current and upcomming version of the Matrix package that would be great? I've checked on mine, but it would be good to check on another system too.

[jeffreyhanson]

Note that you'll need to maually insert this code:

options(Matrix.warnDeprecatedCoerce = n) # where n >= 1

at this line (https://github.com/prioritizr/prioritizr/blob/master/R/zzz.R#L9) to toggle on the warnings during tests/checks with the upcomming Matrix package version.

[codecov[bot]]

Codecov Report

Merging #248 (36f34aa) into master (9e3e825) will decrease coverage by 0.56%. The diff coverage is 96.52%.

:exclamation: Current head 36f34aa differs from pull request most recent head f61f364. Consider uploading reports for the commit f61f364 to get more accurate results

@@            Coverage Diff             @@
##           master     #248      +/-   ##
==========================================
- Coverage   98.53%   97.97%   -0.57%     
==========================================
  Files         129      129              
  Lines        6340     6355      +15     
==========================================
- Hits         6247     6226      -21     
- Misses         93      129      +36     

Impacted Files	Coverage Δ
R/add_boundary_penalties.R	97.29% <50.00%> (ø)
R/add_linear_constraints.R	90.00% <66.66%> (ø)
R/add_linear_penalties.R	90.80% <66.66%> (ø)
R/add_feature_contiguity_constraints.R	91.86% <80.00%> (+0.09%)	:arrow_up:
R/add_asym_connectivity_penalties.R	100.00% <100.00%> (ø)
R/add_cbc_solver.R	100.00% <100.00%> (ø)
R/add_connectivity_penalties.R	100.00% <100.00%> (ø)
R/add_contiguity_constraints.R	100.00% <100.00%> (ø)
R/add_lpsymphony_solver.R	100.00% <100.00%> (ø)
R/add_neighbor_constraints.R	100.00% <100.00%> (ø)
... and 10 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[ricschuster]

@ricschuster, also, if you could please double check that the package works on your system with the current and upcomming version of the Matrix package that would be great? I've checked on mine, but it would be good to check on another system too.

Thanks very much @jeffreyhanson! I've checked the package with both the current and upcoming version of Matrix and it works with both on my system as well.

Once you hear back from the ROpenSci slack we should be good to go.

[ricschuster]

I haven't really done a code review, just spot checks.

[jeffreyhanson]

Awesome - thanks @ricschuster! Could you please check the package on WinBuilder and MacBuilder (https://mac.r-project.org/macbuilder/submit.html) too if that's ok?

[ricschuster]

I can't build the package on Win because I don't have the cplexAPI package on my Win machine which is required to build the vignettes (at least package_overview). Do you have a build that I could submit to WinBuilder and MacBuilder? (I currently don't have a Linux box properly setup)

[jeffreyhanson]

Sorry, I completely forgot to send you a build - I'll do that now.

[ricschuster]

Awesome - thanks @ricschuster! Could you please check the package on WinBuilder and MacBuilder (https://mac.r-project.org/macbuilder/submit.html) too if that's ok?

Checks done and successful. Two notes each.

Common one:

checking package dependencies ... NOTE
Packages suggested but not available for checking:
  'gurobi', 'rcbc', 'cplexAPI', 'lpsymphony'

MacOS:

* checking installed package size ... NOTE
  installed size is 22.5Mb
  sub-directories of 1Mb or more:
    R      1.1Mb
    doc    4.9Mb
    libs  15.1Mb

Win

* checking CRAN incoming feasibility ... NOTE
Maintainer: 'Richard Schuster <[email protected]>'

Possibly mis-spelled words in DESCRIPTION:
  CPLEX (19:9)
  Gurobi (16:49, 18:5)
  MILP (7:18)
  optimality (12:5)
  pre (11:62)

Unknown, possibly mis-spelled, fields in DESCRIPTION:
  'Remotes'

All good to go I'd say. What do you think?

[jeffreyhanson]

Just got another email from the Matrix package maintainers that they've got an updated version, we should probably check that it still works with the new version before releasing to CRAN? I'll start testing it on my computer now.

[ricschuster]

Is that another version that's different to the one we already tested against? I kind of assumed it was the same version. If its different, I completely agree that we should test it before pushing anything to CRAN.

[jeffreyhanson]

Yeah, they made a new version (1.5-0) and we previously tested against an older dev version (1.4-2). The CRAN version is 1.4-1. I found a failure when testing against 1.5-0, so I'll push an update later today with a fix.

[ricschuster]

Thanks very much Jeff!

[jeffreyhanson]

No worries! I've just pushed an update with fixes compatibility with Matrix 1.5-0, could you please test it on your computer?

[ricschuster]

Thanks Jeff! Tests (both local and on Win and Mac builder) are done and got the same notes as I got in the previous tests.

[jeffreyhanson]

Awesome - thanks!

[jeffreyhanson]

Do we know if Matrix 1.5-0 is the "final" developmental version that will be uploaded to CRAN, or is there going to be another developmental version (e.g. 1.5-1) or something?

[jeffreyhanson]

I've just replied to the email from the Matrix maintainers to clarify if 1.5-0 will be the one going to CRAN, or if we should wait for another version. Re-reading the email, it says they plan on releasing an updated Matrix package on Sept 8th.

[ricschuster]

Thanks Jeff. The Sept 8 plan was for 1.4.2 as far as I can tell.

From a previous email: "We are hoping to release Matrix 1.4-2 to CRAN in early September"

Will be good to hear about 1.5-0 or an even newer version.

[jeffreyhanson]

@ricschuster, I've just updated the PR with updates to address #218. Could you please take a look? The main changes are in R/boundary_matrix.R and tests/testthat/boundary_matrix.R.

[ricschuster]

Thanks Jeff! Looks good to me.

The only small cosmetic thing I've noticed is in test_boundary_matrix.R on lines 144 and 145 where you use:

  x <- boundary_matrix(sf::st_as_sf(d))
  y <- boundary_matrix(d)

instead of the 'usual'

  x <- boundary_matrix(d)
  y <- boundary_matrix(sf::st_as_sf(d))

[jeffreyhanson]

Thanks for catching that! Since this test is focussed more on sf, I thought it would make since for x to test the sf object. I'll push a commit in a moment to make the test clearer.

[ricschuster]

Thanks Jeff!

[jeffreyhanson]

Just heard back from the Matrix maintainers, they don't plan on making any further changes that aren't fully backwards compatible with the last dev version they mentioned (i.e. 1.5-0). Did you manage to get on the R devel emailing list to ask about arhived packages?

[ricschuster]

Great! Still haven't gotten conformation from the mailing list. Should we just go ahead and try with CRAN?

[jeffreyhanson]

Yeah - I guess so. Was there anything else we needed to do before merging the PR?

[jeffreyhanson]

If not, I'll squash and merge the PR?

[jeffreyhanson]

After I've merged the PR, it would also probably be worth waiting till the github actions have checked everything's good and then running the package again through winbuilder and macbuilder again to avoid any issues with the submission - how does that sound?

[jeffreyhanson]

Thanks - good catch with that typo!

[ricschuster]

Just made a tiny change to the cran comments. Other than that everything is good on my end.

After you merge we can wait for the actions to complete and then submit to the builders again before submitting.