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

Steepest edge #1

Merged
merged 32 commits into from
Jul 31, 2017
Merged

Steepest edge #1

merged 32 commits into from
Jul 31, 2017

Conversation

rachellim
Copy link

Implemented steepest edge pivot selection strategy (new selection rule in SteepestEdge.h; also modified Tableau to update data relevant to steepest edge before pivot operation)

rachellim added 25 commits July 13, 2017 11:18
…leau; implemented initialization. TODO: tests, implement update
…leau; implemented initialization. TODO: tests, implement update
@rachellim rachellim requested a review from guykatzz July 19, 2017 18:57
@@ -82,6 +82,8 @@ class ITableau
virtual void computeMultipliers() = 0;
virtual void computeReducedCost( unsigned nonBasic ) = 0;
virtual const double *getCostFunction() const = 0;
// TODO: not sure if i'm allowed to add to this virtual class?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine

* can update gamma more cheaply using a recurrence relation. See Goldfarb and Reid (1977),
* Forrest and Goldfarb (1992).
*/
return (c[j] * c[j]) / gamma[j];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure gamma[j] isn't zero?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't be, because it's the weight of the vector p[j] (the direction the variable x moves in if you increment nonbasic variable Xj). We can add an ASSERT to be sure?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, let's add an assert

_basisFactorization->forwardTransformation( ANColumnQ, invB_Aq );

// Compute alphas and nus
double *alpha = new double[_n-_m];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This memory needs to be deleted, I think.
If this is reallocated in every iteration, we can just allocate it once when the dimensions of the tableau are set, as "work memory". If we do that, we need to adjust their size when addRow() is invoked.

@guykatzz
Copy link
Collaborator

Please check if we need to update the gamma function in Tableau::performDegeneratePivot

@rachellim rachellim merged commit 4f384cd into master Jul 31, 2017
@rachellim rachellim deleted the steepest-edge branch July 31, 2017 16:10
jjgold012 added a commit that referenced this pull request Nov 28, 2019
alexopenu added a commit that referenced this pull request Mar 3, 2020
merging from guykatzz/master
yuvaljacoby pushed a commit to yuvaljacoby/Marabou-1 that referenced this pull request May 23, 2020
wu-haoze added a commit that referenced this pull request Aug 21, 2020
Changes towards satisfying the unit tests and runtime assertions
guyam2 pushed a commit that referenced this pull request Feb 23, 2021
matanost pushed a commit that referenced this pull request Nov 2, 2021
@lucaslulu lucaslulu mentioned this pull request Mar 7, 2022
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.

2 participants