Skip to content

convert lists to arrays for tuple indexing#1077

Merged
bqpd merged 7 commits intomasterfrom
fixvecsub
Apr 18, 2017
Merged

convert lists to arrays for tuple indexing#1077
bqpd merged 7 commits intomasterfrom
fixvecsub

Conversation

@bqpd
Copy link
Copy Markdown
Contributor

@bqpd bqpd commented Apr 13, 2017

closes convexengineering/SPaircraft#37

@whoburg, ready for review

@bqpd
Copy link
Copy Markdown
Contributor Author

bqpd commented Apr 13, 2017

todo: implement this or something similar as a test:

from gpkit import *
from gpkit.constraints.relax import ConstantsRelaxed

x = VectorVariable(3, "x")
y = VectorVariable(3, "y")
ymax = VectorVariable(3, "ymax")

with SignomialsEnabled():
    m = Model(x.prod(), [x + y >= 1, y <= ymax])

m.substitutions.update({"ymax": [0.3, 0.5, 0.8]})
print m.localsolve().summary()

@mayork
Copy link
Copy Markdown
Contributor

mayork commented Apr 14, 2017

@whoburg it'd really help us do some much needed clean up in the D8 code if this gets merged

gpkit/keydict.py Outdated
# KeyDict values are expected to be immutable (Numbers)
# or to have a copy attribute.
v = v.copy()
if (isinstance(k, VarKey) and k.shape
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

move isinstance call to the end of the list of conditions, for speed?

gpkit/keydict.py Outdated
# or to have a copy attribute.
v = v.copy()
if (isinstance(k, VarKey) and k.shape
and not k.idx and not isinstance(v, np.ndarray)):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

inline comment desperately needed.

@whoburg
Copy link
Copy Markdown
Collaborator

whoburg commented Apr 14, 2017

added @bqpd's suggested test.

@whoburg
Copy link
Copy Markdown
Collaborator

whoburg commented Apr 14, 2017

I set the review status to approved so that @bqpd can merge, but please add the inline comment first

@mayork
Copy link
Copy Markdown
Contributor

mayork commented Apr 17, 2017

@bqpd updates?

@bqpd bqpd merged commit 6292360 into master Apr 18, 2017
@bqpd bqpd deleted the fixvecsub branch April 18, 2017 23:41
@bqpd
Copy link
Copy Markdown
Contributor Author

bqpd commented Apr 18, 2017

@mayork, it's merged!

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

sub for a vector variable? (and, fix documentation)

3 participants