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

r-hub builder prioritizr 3.0.1: ERROR #40

Closed
ricschuster opened this Issue Nov 7, 2017 · 14 comments

Comments

Projects
None yet
4 participants
@ricschuster
Copy link
Member

ricschuster commented Nov 7, 2017

Build ID: prioritizr_3.0.1.tar.gz-610e8ddbf5f4e08d4e406bb25a2120e8
Platform: Debian Linux, R-devel, GCC
Submitted: 27 minutes 55.3 seconds ago
Build time: 26 minutes 59.6 seconds
ERRORS:

  • checking tests ...
    Running ‘testthat.R’
    ERROR
    Running the tests in ‘tests/testthat.R’ failed.
    Last 13 lines of output:

    • Optimal Solution Found *

    Solution Found: Node 0, Level 0
    Solution Cost: 4.0000000000
    list()
    attr(,"class")
    [1] "Waiver"
    testthat results ================================================================
    OK: 8428 SKIPPED: 47 FAILED: 1

    1. Failure: solution (compressed formulation) (@test_add_max_cover_objective.R#48)

    Error: testthat unit tests failed
    Execution halted

NOTES:

  • checking package dependencies ... NOTE
    Packages suggested but not available for checking: ‘gurobi’ ‘Rsymphony’

  • checking installed package size ... NOTE
    installed size is 6.2Mb
    sub-directories of 1Mb or more:
    doc 3.4Mb
    libs 1.7Mb

Complete log

@jeffreyhanson

This comment has been minimized.

Copy link
Contributor

jeffreyhanson commented Nov 9, 2017

I've worked out why this is happening. It looks like lpsymphony has a bug where it will ignore upper bounds on binary variables if the problem is solved during the presolve step. This causes prioritizr to yield incorrect solutions when solving problems with add_lpsymphony_solver(). I suspect this might be occurring because lpsymphony uses an old version of SYMPHONY.

@jeffreyhanson

This comment has been minimized.

Copy link
Contributor

jeffreyhanson commented Nov 9, 2017

How do you think we should address this issue?

@jeffreyhanson

This comment has been minimized.

Copy link
Contributor

jeffreyhanson commented Nov 9, 2017

Here's a reproducible example.

# define lpsymphony parameters
p <- structure(list(time_limit = -1L, first_feasible = FALSE, gap_limit = 0, 
    verbosity = 1), .Names = c("time_limit", "first_feasible", 
"gap_limit", "verbosity"))

# define optimisation problem
model <- structure(list(obj = c(1e-10, 2e-10, 4e-10, 1, 1), mat = structure(c(1, 
1, 1, 1, 0, 2, 1, 1, 4, -1, 0, 0, 0, -1, 0), .Dim = c(3L, 5L), .Dimnames = list(
    NULL, NULL)), dir = c(">=", ">=", "<="), rhs = c(0, 0, 3), 
    types = c("B", "B", "B", "B", "B"), bounds = structure(list(
        lower = structure(list(ind = 1:5, val = c(0, 0, 0, 0, 
        0)), .Names = c("ind", "val")), upper = structure(list(
            ind = 1:5, val = c(0, 1, 1, 1, 1)), .Names = c("ind", 
        "val"))), .Names = c("lower", "upper")), max = TRUE), .Names = c("obj", 
"mat", "dir", "rhs", "types", "bounds", "max"))

# show upper bounds on decision variables
print(model$bounds$upper$val)
#> [1] 0 1 1 1 1

# solve problem
s <- do.call(lpsymphony::lpsymphony_solve_LP, append(model, p))

# extract solution
print(s$solution)
#> [1] 1 1 0 1 1

As we can see, the solution has a 1 for the first variable in the problem even though it has an upper bound of zero. Thus lpysmhpony has a bug in it.

@ricschuster

This comment has been minimized.

Copy link
Member

ricschuster commented Nov 9, 2017

Thanks Jeff.
First, I think we should alert the lpsymphony developers to this. If they can figure out a quick fix we might not have to do anything on the prioritizr side.

If they say it might take them a while to figure this out we might want to pull lpsymphony support for now. What do you think?

I'm going to send the lpsymphony developer an email now.

@mstrimas

This comment has been minimized.

Copy link
Contributor

mstrimas commented Nov 9, 2017

lpsymphony seems mostly redundant anyway given that Rsymphony does the same thing. Is there some benefit to lpsymphony that I'm not recalling? That said, this issue is very unlikely to arise in practice. It requires that the user has locked out planning units and that the problem is simple enough to be solved by the presolver, which is unlikely in anything but these simple, unrealistic test cases.

@ricschuster ricschuster added bug and removed bug labels Nov 9, 2017

@jeffreyhanson

This comment has been minimized.

Copy link
Contributor

jeffreyhanson commented Nov 10, 2017

Thanks for emailing the lpsymphony developer @ricschuster.

Yeah, that's true @mstrimas, this issue is unlikely to occur in the wild.

I think the reason we included lpsymphony is because it should be easier to install on Windows since it includes a built-in version of SYMPHONY.

@jeffreyhanson jeffreyhanson added the bug label Nov 10, 2017

@ricschuster

This comment has been minimized.

Copy link
Member

ricschuster commented Nov 10, 2017

re: reason for inclusion. Very interesting. I always thought it was because of Linux users. Rsymphony is easy to install on Windows and doesn't need any extra steps or need to install SYMPHONY. Maybe supporting Rsymphony and not both would be sufficient?

@jeffreyhanson

This comment has been minimized.

Copy link
Contributor

jeffreyhanson commented Nov 10, 2017

lol that is interesting indeed.

On Linux, Rsymphony needs to have SYMHPONY installed, but this is easy to do on Ubuntu using the Ubuntu system package manager. Perhaps, installing Rsymphony on other Linux distros is more challenging?

Does SYMPHONY come bundled automatically in the Windows binary version of Rsymphony? I can't find it in the source (https://github.com/cran/Rsymphony)

What's it like installing Rsymphony on Mac OSX?

jeffreyhanson added a commit that referenced this issue Nov 10, 2017

@jeffreyhanson

This comment has been minimized.

Copy link
Contributor

jeffreyhanson commented Nov 10, 2017

The unit tests should be configured to skip tests that involve solving problems when run using web services so that the tests can be completed in a feasible amount of time. So, based on this premise, the latest commit fixes the issue wherein this test is run on rhub and CRAN.

However, the bug uncovered on rhub, resulting from a bug in lpsymphony still remains, and so I have left this issue open until that is fixed.

@ricschuster

This comment has been minimized.

Copy link
Member

ricschuster commented Nov 10, 2017

I remember having some trouble installing SYMPHONY when I first started using Linux Mint, but that was most likely my lack of experience with Linux.

Not sure how it works with Rsymphony on Windows, but I know that I didn't need to install SYMPHONY on the last two Win10 versions I installed the Rsymphony package on. Just to make sure I did tried it again just now on a Win10 machine that I haven't installed SYMPHONY on and it worked fine.

@mstrimas

This comment has been minimized.

Copy link
Contributor

mstrimas commented Nov 10, 2017

Back when I first started looking at these open source solvers a year and a half ago, I remember Rsymphony being a real pain to install on Mac OS. However, when I installed it yesterday on a new computer it was no problem at all. Not sure what changed, but it seems Rsymphony is now easy to install on all three systems and doesn't have the issue with the bug that lpsymphony has, so is probably the open source solver to recommend going forward.

@momeni133

This comment has been minimized.

Copy link

momeni133 commented Nov 10, 2017

Dear Matt
Could you please share installation process on windows and Linux?

@ricschuster

This comment has been minimized.

Copy link
Member

ricschuster commented Nov 10, 2017

Windows: in R just use install.packages("Rsymphony"), that's all you need.

@jeffreyhanson

This comment has been minimized.

Copy link
Contributor

jeffreyhanson commented Jan 27, 2018

I think we can close this issue now because prioritizr throws a warning if an older version of lpsymphony is used which contains the bug which returns infeasible solutions (c3d0fa3)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment