Opened 6 years ago
Closed 5 years ago
#18763 closed enhancement (fixed)
COIN backend should support basis status and tableau data functions
Reported by: | mkoeppe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.9 |
Component: | numerical | Keywords: | lp |
Cc: | yzh, ncohen | Merged in: | |
Authors: | Yuan Zhou | Reviewers: | Vincent Delecroix |
Report Upstream: | N/A | Work issues: | |
Branch: | c2488a8 (Commits, GitHub, GitLab) | Commit: | c2488a8cf075664de211996ab3360ee8f4d48e56 |
Dependencies: | Stopgaps: |
Description (last modified by )
This is for COIN backend support in #18733 and #18688.
Should add
(1) getBasisStatus
, setBasisStatus
,
(2) getBInvARow
, getBInvACol
, getBasics
,
to src/sage/numerical/backends/coin_backend.{pxd,pyx}
Possible that one needs to add other functions as well, such as enableSimplexInterface
...
Reference to the relevant Clp header file: https://projects.coin-or.org/Osi/browser/trunk/Osi/src/OsiClp/OsiClpSolverInterface.hpp?rev=1472
Change History (16)
comment:1 Changed 6 years ago by
- Description modified (diff)
comment:2 Changed 6 years ago by
- Branch set to u/yzh/coin_backend_should_support_basis_status_and_tableau_data_functions
comment:3 Changed 6 years ago by
- Commit set to 9fdbd841cbd1585bca4633865e28440ad2ee5df0
comment:4 Changed 6 years ago by
- Commit changed from 9fdbd841cbd1585bca4633865e28440ad2ee5df0 to 3644950e0be45eb124c55931b57098415bbe7b21
Branch pushed to git repo; I updated commit sha1. New commits:
3644950 | Add CoinBackend::get_binva_row, get_binva_col
|
comment:5 Changed 6 years ago by
- Status changed from new to needs_review
comment:6 follow-up: ↓ 7 Changed 5 years ago by
- Reviewers set to Vincent Delecroix
- Status changed from needs_review to needs_work
Please fill the Authors
field with your full name.
Instead of sage_malloc
you could use check_malloc
(that checks that the memory is indeed allocated and raises an error if not).
Otherwise, everything looks clean and tests run smoothly on my computer (with Coin installed).
Vincent
comment:7 in reply to: ↑ 6 ; follow-up: ↓ 9 Changed 5 years ago by
Instead of
sage_malloc
you could usecheck_malloc
(that checks that the memory is indeed allocated and raises an error if not).
Thanks a lot for reviewing the ticket.
Does one call sage_free
to free the memory allocated by check_malloc
?
Since sage_malloc
is used throughout the file, I was trying to be consistent. Would you suggest replacing them all by check_malloc
? Or perhaps better to update them using #18868?
comment:8 Changed 5 years ago by
comment:9 in reply to: ↑ 7 ; follow-up: ↓ 11 Changed 5 years ago by
Hi Yuan,
Replying to yzh:
Instead of
sage_malloc
you could usecheck_malloc
(that checks that the memory is indeed allocated and raises an error if not).Thanks a lot for reviewing the ticket. Does one call
sage_free
to free the memory allocated bycheck_malloc
? Sincesage_malloc
is used throughout the file, I was trying to be consistent. Would you suggest replacing them all bycheck_malloc
? Or perhaps better to update them using #18868?
I do not think that #18868 is good idea in this case. You will create a Python object for nothing.
check_malloc
is equivalent to sage_malloc
except that it will raise an error in case the memory was not allocated (where sage_malloc
just returns NULL
). So the memory should be freed with sage_free
. You can safely replace occurrences of sage_malloc
by check_malloc
. Moreover you can always replace
my_ptr = sage_malloc(12) if my_ptr == NULL: raise MemoryError("allocation failed")
by
my_ptr = check_malloc(12)
Vincent
comment:10 Changed 5 years ago by
- Commit changed from 3644950e0be45eb124c55931b57098415bbe7b21 to 3ab6b33cab8eee152a417f4db45d5fe619fc797a
Branch pushed to git repo; I updated commit sha1. New commits:
3ab6b33 | replace sage_malloc by check_malloc
|
comment:11 in reply to: ↑ 9 Changed 5 years ago by
- Status changed from needs_work to needs_review
Thank you, Vincent.
I changed sage_malloc
to check_malloc
.
comment:12 Changed 5 years ago by
- Status changed from needs_review to needs_work
The patchbot reported some errors (when cbc is not installed)
sage -t --long src/sage/numerical/backends/coin_backend.pyx ********************************************************************** File "src/sage/numerical/backends/coin_backend.pyx", line 1260, in sage.numerical.backends.coin_backend.CoinBackend.get_basis_status Failed example: p.solve() ... NameError: name 'p' is not defined ********************************************************************** File "src/sage/numerical/backends/coin_backend.pyx", line 1285, in sage.numerical.backends.coin_backend.CoinBackend.get_basis_status Failed example: lp_coin = lp.get_backend() ... NameError: name 'lp' is not defined **********************************************************************
comment:13 Changed 5 years ago by
- Commit changed from 3ab6b33cab8eee152a417f4db45d5fe619fc797a to c2488a8cf075664de211996ab3360ee8f4d48e56
Branch pushed to git repo; I updated commit sha1. New commits:
c2488a8 | specify optional-cbc in doctests
|
comment:14 Changed 5 years ago by
- Status changed from needs_work to needs_review
Oops, I forgot to indicate optional - cbc at some places
comment:15 Changed 5 years ago by
- Milestone changed from sage-6.8 to sage-6.9
- Status changed from needs_review to positive_review
Good to go!
comment:16 Changed 5 years ago by
- Branch changed from u/yzh/coin_backend_should_support_basis_status_and_tableau_data_functions to c2488a8cf075664de211996ab3360ee8f4d48e56
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Fix bugs in CoinBackend::get/set_basis_status(); Add doctests