Skip to content
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

Improve lp_sum #66

Merged
merged 1 commit into from
Feb 10, 2021
Merged

Improve lp_sum #66

merged 1 commit into from
Feb 10, 2021

Conversation

lovasoa
Copy link
Contributor

@lovasoa lovasoa commented Feb 10, 2021

Fixes #65
Fixes #41

Helps with #37

@jcavat jcavat merged commit 5f59fef into rust-or:master Feb 10, 2021
@lovasoa lovasoa deleted the better_lp_sum branch February 10, 2021 21:20
@lovasoa
Copy link
Contributor Author

lovasoa commented Feb 10, 2021

Oh, I should have closed that. I opened that to work around the fact that processing the tree was recursive and failed on deep trees. Now that we have flat trees, it isn't needed so much. Plus, I think simplify actively breaks the balanced tree:
https://github.com/jcavat/rust-lp-modeler/blob/master/src/dsl/variables.rs#L825

The real problem is that there is no way to represent a sum of more than two elements internally.

@jcavat
Copy link
Collaborator

jcavat commented Feb 11, 2021

yes, that's true. Maybe #67 is still relevant after all. That could be resolved by a different representation disscussed here: #37 (comment). But as you said, it is more ambitious.

@lovasoa
Copy link
Contributor Author

lovasoa commented Feb 11, 2021

Yes #67 is still totally relevant. For medium or large problems, rust-lp-modeler takes much longer to create the problem than it takes to actually solve it !

I am working on #37 (comment) , but as a separate crate, as I want to be able to make the right design decisions without worrying about backwards compatibility. I'll let you know when I have something to show.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lp_sum creates a linked list instead of a balanced tree lp-sum panics on empty vector
2 participants