Opened 9 years ago
Closed 9 years ago
#7561 closed defect (fixed)
Replaces InfinitePolynomialRing in MixedIntegerLinearProgram by 'var', and bug fixing in constraints()
Reported by: | ncohen | Owned by: | jkantor |
---|---|---|---|
Priority: | major | Milestone: | sage-4.3 |
Component: | numerical | Keywords: | |
Cc: | Merged in: | sage-4.3.rc0 | |
Authors: | Nathann Cohen | Reviewers: | Martin Albrecht |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
InfinitePolynomialRing? was responsible for some bugs and extreme slowness in the utilisation of MixedINtegerLinearProgram for LP containing more than 1000 variables.
By replacing this polynomial ring by 'var', this is settled and waaaaayyyy mroe efficient !!
One simple bug in constraints() is also fixed in this patch. A nasty -1 was shifting all the constraints compared to their bounds. This only affected the functions show() and constraints() and is of no incidence on the solve() function.
This patch depends on #7270
Attachments (2)
Change History (12)
comment:1 Changed 9 years ago by
- Description modified (diff)
- Status changed from new to needs_review
- Summary changed from Replaces InfinitePolynomialRing in MixedIntegerLinearProgram by 'var' to Replaces InfinitePolynomialRing in MixedIntegerLinearProgram by 'var', and bug fixing in constraints()
comment:2 Changed 9 years ago by
comment:3 follow-up: ↓ 5 Changed 9 years ago by
Hello !!! It seems there are no "id" anymore in the file, and I corrected the x[3]..
The min/max variables are only defined in some (short) functions where they appear natural enough... Do you think we should change the arguments of the add_constraint() function ? I understand why you do not like to see this and I was not aware of it before you mentionned it, but they seem mostly harmless and I have no idea of which keyword we could pick to replace them... If you have any idea, though.... :-)
Nathann
comment:4 Changed 9 years ago by
Still around ??? :-)
It would be good if this patch was merged into next release ! :-)
comment:5 in reply to: ↑ 3 Changed 9 years ago by
Replying to ncohen:
Hello !!! It seems there are no "id" anymore in the file, and I corrected the x[3]..
Hi, it seems you didn't upload your new patch.
The min/max variables are only defined in some (short) functions where they appear natural enough... Do you think we should change the arguments of the add_constraint() function ? I understand why you do not like to see this and I was not aware of it before you mentionned it, but they seem mostly harmless and I have no idea of which keyword we could pick to replace them... If you have any idea, though.... :-)
How about minimum
& maximum
? I'll leave it to you to change it or to keep min/max.
comment:6 Changed 9 years ago by
I forgot to uploaded it and erased it since... I'll upload a new one in a few seconds ! :-)
comment:7 Changed 9 years ago by
Here it is ! I only corrected the x[3] and let the min/max be... The function in which they appear is very short, and longer version would mean updating manymany patches (currently under review) and having longer aliases when it does not hurt that much. ( though I understand how you felt about them, I did not realized it when I first used them ).
We will be able to change them later if needed anyway :-)
Nathann
Changed 9 years ago by
comment:8 Changed 9 years ago by
- Status changed from needs_review to positive_review
This should go into 4.3, it opens a whole new world for MIP problems.
comment:9 Changed 9 years ago by
Thankssssssssssssss :-)
Changed 9 years ago by
comment:10 Changed 9 years ago by
- Merged in set to sage-4.3.rc0
- Resolution set to fixed
- Reviewers set to Martin Albrecht
- Status changed from positive_review to closed
Patch looks good and indeed speeds up everything considerably for me. Some small issues:
# Returns the optimal value of x[3]
but you are asking for y[2][9]