convexengineering / gplibrary

Useful subsystem models
MIT License
10 stars 11 forks source link

initial transfer of LG model from gpjet #27

Closed pgkirsch closed 8 years ago

pgkirsch commented 8 years ago

Ready for review @whoburg

Also, I added a constraint and the result got much closer to the actual 737! Unfortunately, I have no idea why it changed the things it did....

whoburg commented 8 years ago

Looks good. a number of equality constraints can be converted to inequalities; otherwise I think this is ready to go.

Also interesting is that for me LandingGear().test() takes about 2 seconds with cvxopt, and under one second with mosek.

whoburg commented 8 years ago

@bqpd I think we're starting to get enough models that it would make sense to have models-gpkit unit tests that just run all the test() methods. Do we already have that in place? How do I run it? Should we ask Marshall to set up Jenkins to run it too?

bqpd commented 8 years ago

I'd say import a testing.py file which imports ask the desired models to top level, then for through dir running everything with a test method. On Jan 13, 2016 05:17, "Warren Hoburg" notifications@github.com wrote:

@bqpd https://github.com/bqpd I think we're starting to get enough models that it would make sense to have models-gpkit unit tests that just run all the test() methods. Do we already have that in place? How do I run it? Should we ask Marshall to set up Jenkins to run it too?

— Reply to this email directly or view it on GitHub https://github.com/aeroa/gpkit-models/pull/27#issuecomment-171077703.

bqpd commented 8 years ago

(smartphone typos, sorry) On Jan 13, 2016 05:34, "Edward Burnell" eburn@mit.edu wrote:

I'd say import a testing.py file which imports ask the desired models to top level, then for through dir running everything with a test method. On Jan 13, 2016 05:17, "Warren Hoburg" notifications@github.com wrote:

@bqpd https://github.com/bqpd I think we're starting to get enough models that it would make sense to have models-gpkit unit tests that just run all the test() methods. Do we already have that in place? How do I run it? Should we ask Marshall to set up Jenkins to run it too?

— Reply to this email directly or view it on GitHub https://github.com/aeroa/gpkit-models/pull/27#issuecomment-171077703.

pgkirsch commented 8 years ago

The only reason I'm hesitant to convert equality constraints to inequalities is because I have a suspicion that it makes them more susceptible to being dual infeasible in the future (if another model is added that puts a stronger pressure in the opposite direction). Do you buy that? The four changes you propose don't affect the result in the present model, but when I debug dual infeasibility, the first thing I do is make all inequality constraints equalities wherever possible, and it can sometimes help.

whoburg commented 8 years ago

@pgkirsch, regarding your points about equality constraints, this illustrates to me that we still need a more formal way to talk about the assumptions/pressures of a model. Until we have that I don't think we can issue a blanket policy on when to use equality constraints.

@bqpd, two questions regarding your suggestion to have a testing.py file import "the desired models" and then dir through everything with a test method:

  1. dumb question, but dir returns a list of strings. Is it obvious how to get the actual python objects from the strings?
  2. Would you do things like from beam import *, trying to require less updates to testing.py, or would you use explicit imports?
bqpd commented 8 years ago

TESTS = []

import beam
TESTS.append(beam)

for module in TESTS:
    for attr_name in dir(module):
        module_attr = getattr(beam, attr_name)
        if hasattr(module_attr, "test"):
            module_attr.test()  # wrap this in a nice error message
whoburg commented 8 years ago

Ah - got it. I originally thought you were suggesting using dir to find the modules.

I'll take a shot at adding the testing script today.

whoburg commented 8 years ago

test() currently fails for me:

---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
<ipython-input-3-b8860f1c5b14> in <module>()
----> 1 lg.test()

/Users/whoburg/MIT/dev/models-gpkit/aircraft/landing_gear.py in test(self)
    249     def test(self):
    250         sol = self.localsolve()
--> 251         npt.assert_almost_equal(sol('x_n') + sol('B'), sol('x_m'), decimal=5)
    252         npt.assert_almost_equal(sol('\Delta x_n') + sol('x_n'), sol('x_{CG}'),
    253                                 decimal=1)

/Users/whoburg/anaconda/envs/gpkit/lib/python2.7/site-packages/numpy/testing/utils.pyc in assert_almost_equal(actual, desired, decimal, err_msg, verbose)
    488         pass
    489     if round(abs(desired - actual), decimal) != 0 :
--> 490         raise AssertionError(_build_err_msg())
    491
    492

AssertionError:
Arrays are not almost equal to 5 decimals
 ACTUAL: 21.284771127378246
 DESIRED: 21.284763621703661
whoburg commented 8 years ago

also, I added the testing script discussed above in #29 -- we can move that conversation there