Prop #157

courtin · 2018-03-25T13:53:53Z

Adding simple propeller model with fixed weight and radius.

mjburton11 · 2018-03-25T14:11:35Z

gpkitmodels/GP/aircraft/prop/propeller.py

+    Variables
+    ---------
+    R          10           [m]         prop radius
+    W          10           [lbf]        prop weight


I don't like the propeller having a fixed weight. Does it add 10 lbs to the solar model too? I would probably leave it without weight variable for now. Maybe it can scale with radius?

Agreed fixed weights are not correct, but I'm not sure the best simple weight model yet. I think we should leave it with a small weight value as a placeholder until we can implement a real weight model. This lets it be added to the aircraft.components list for integration into the overall model without adding an inaccurate weight estimate.

mjburton11 · 2018-03-25T14:14:21Z

gpkitmodels/GP/aircraft/prop/propeller.py

+    def setup(self):
+        exec parse_variables(Propeller.__doc__)
+
+    def performance(self,state):


Instead of declaring it as a model I would declare like:

""" docstring """ performance = Propeller_Performance def setup(self): ...

Also can we rename it from performance to flight_model so it's consistent with wing.py and the other GP models in this repo?

mjburton11 · 2018-03-25T14:16:21Z

gpkitmodels/GP/aircraft/prop/propeller.py

+                lam == V/(omega*R),
+                CT == Tc*lam**2,
+                CP == Q*omega/(.5*rho*(omega*R)**3*math.pi*R**2),
+                eta == CT*lam/CP, #Is this the same eta? check 


Is this calculating the same eta? If not which equation is driving it and which is correct?

This is for the motor weight estimate; it should be left off this pull request.

mjburton11 · 2018-03-25T14:17:02Z

gpkitmodels/GP/aircraft/prop/qprop.py

@@ -0,0 +1,58 @@
+" qprop model "


Is this model done yet? If not let's leave it off of this PR

mjburton11 · 2018-03-25T14:17:34Z

gpkitmodels/GP/aircraft/prop/prop_test.py

+from qprop import QProp
+from gpkitmodels.GP.aircraft.wing.wing_test import FlightState
+
+def eta_test():


Can this test be a unit test for the Propeller model?

…nto prop

bqpd · 2018-03-26T16:09:57Z

gpkitmodels/GP/aircraft/prop/prop_test.py

@@ -0,0 +1,31 @@
+" propelle tests "


bqpd · 2018-03-26T16:10:09Z

gpkitmodels/GP/aircraft/prop/propeller.py

+    Q                       [N*m]       torque
+    omega                   [rpm]       propeller rotation rate
+
+    """


It'd be great to have bounds as well!

bqpd · 2018-03-27T15:55:08Z

it looks like this is legitimately not passing tests, I'll take a look

bqpd · 2018-03-27T15:56:03Z

gpkitmodels/GP/aircraft/prop/prop_test.py

+def eta_test():
+
+    fs = FlightState()
+    p = Propeller(fs)


propeller does not accept flight state as an argument

bqpd · 2018-03-27T15:56:56Z

@mjburton11 @courtin see code review above (deleting comments related to #24 to make this PR easier to read)

bqpd · 2018-03-28T22:43:23Z

It's 19/40/40/a very lucky 60/an unlucky 60/25/36/unlucky 60 on gpkit models testing, so it's nearly the same

galbramc · 2018-03-28T23:36:59Z

What about the script?

bqpd · 2018-03-28T23:37:39Z

The script is very similar on that test

galbramc · 2018-03-29T01:45:32Z

But can it be the same? Are the difference needed? If the differences do not matter we can add it to the templating for the others so we have one less script to maintain.

bqpd · 2018-03-29T01:49:56Z

One of them needs to install local gpkit and master gplibrary, the other needs to the the other, and they call test_repos with one flag different so that gpklibrary is not reinstalled...I think they need to be separate unfortunately.

galbramc · 2018-03-29T02:42:51Z

No not those. Are these differences signifincant:

python -c "from gpkit.tests.test_repo import test_repo; test_repo(xmloutput=True)"

vs

python $PIP install -v --no-cache-dir --no-deps -e $WORKSPACE
echo "from gpkit.tests.test_repo import test_repo; test_repo('./.', xmloutput=True)" > jenkins_test.py
python $COVERAGE run --append --source=. jenkins_test.py

bqpd · 2018-03-29T02:45:27Z

Oh! Sorry! I missed that you meant gpkit_ResearchModel_gplibrary_PR, though I see that now when scanning back up. You're totally right, it can inherit from gpkit_ResearchModel. Although we might need to add that python $PIP install -v --no-cache-dir --no-deps -e $WORKSPACE line to ResearchModel if it's not already in there.

galbramc · 2018-03-29T02:54:12Z

Should all research models be installed? I can't tell if the prop failure is due to this PR or due to the lack of the install.

bqpd · 2018-03-29T02:56:47Z

The prop failure is definitely due to this PR, since this PR is what introduces that test.

galbramc · 2018-03-29T02:57:20Z

So if you decide you need to install just the gplibrary, you can add an if test to the gpkit_RearchModel template just for gplibrary test. That is preferable rather than having one more script with a tiny perturbation.

bqpd · 2018-03-29T03:05:49Z

@galbramc I'm afraid I'm not following what you're saying! Are you proposing a further modification?

mjburton11 · 2018-03-29T13:28:14Z

I don't understand why it think prop is not a module...

Do we just need to increase the run time of the research tests? That seems to be the only reason it is failing.

bqpd · 2018-03-29T15:47:46Z

increased the timeout, though it's too bad it takes so long!

mjburton11 · 2018-03-29T15:49:04Z

agreed. still no understanding of why TESTS is failing??

mjburton11 · 2018-03-29T15:49:09Z

test models please

mjburton11 · 2018-03-29T17:18:25Z

Woohoo! Passed research tests! 1 to @bqpd do you know what is going on?

galbramc · 2018-03-30T14:40:58Z

I added more cores to the Windows 7 VM. Hopefully it runs faster now.

mjburton11 · 2018-03-30T14:42:11Z

@galbramc do you know why this is still failing TESTS?

bqpd · 2018-03-30T15:01:13Z

That might be my mistake; I may have been too hasty in assuming the differences above were not significant!

…

On Fri, Mar 30, 2018, 10:42 Michael Burton ***@***.***> wrote: @galbramc <https://github.com/galbramc> do you know why this is still failing TESTS? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#157 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABagGPNwS3uzSnJPWsCEb2IWwam_kXQJks5tjkRDgaJpZM4S6JcH> .

galbramc · 2018-03-30T16:10:39Z

So does this library and no other need to be installed? What happens if the others are installed?

bqpd · 2018-03-30T16:17:36Z

Or this library needs to not be installed...I'll take a look. The fix might well be a little more logic in gpkit

mjburton11 · 2018-03-30T16:37:02Z

test this please

bqpd · 2018-03-31T02:01:44Z

test this please

mjburton11 · 2018-04-02T13:27:05Z

test this please

mjburton11 · 2018-04-02T15:14:42Z

@bqpd why aren't tests running?

mjburton11 · 2018-04-02T15:30:17Z

test models please

mjburton and others added 6 commits February 22, 2018 21:40

initial propeller efficiency model

ca4d658

modify tests

875e650

initial qprop

a06db84

Add prop structure and performance models

f30e2ea

Add prop structure and performance models

129cd22

Updated propeller to base and performance class

eb7c39d

courtin requested a review from mjburton11 March 25, 2018 13:53

mjburton11 suggested changes Mar 25, 2018

View reviewed changes

mjburton and others added 8 commits March 25, 2018 10:20

update units tests to include prop model

9bfa23c

Merge branch 'prop' of https://github.com/convexengineering/library i…

4d2da32

…nto prop

Changes per mike's review

3b2c57b

Merge branch 'prop' of github.com:convexengineering/gplibrary into prop

1840d3e

small changes

195883c

remove qprop

602606f

Merge branch 'master' into prop

0b3c8ae

add static model in

c47db82

mjburton11 approved these changes Mar 26, 2018

View reviewed changes

bqpd reviewed Mar 26, 2018

View reviewed changes

add missing __init__.py

05cbb85

bqpd requested changes Mar 27, 2018

View reviewed changes

convexengineering deleted a comment from mjburton11 Mar 27, 2018

bqpd mentioned this pull request Mar 27, 2018

failing tests convexengineering/solar#24

Closed

convexengineering deleted a comment from acdl-jenkins Mar 29, 2018

add fit csvs to setup.py (#159)

db5e4cb

mjburton11 merged commit bb4451e into master Apr 3, 2018

mjburton11 deleted the prop branch April 3, 2018 19:07

Prop #157

Prop #157

Conversation

courtin commented Mar 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bqpd commented Mar 27, 2018

Choose a reason for hiding this comment

bqpd commented Mar 27, 2018 • edited

bqpd commented Mar 28, 2018 • edited

galbramc commented Mar 28, 2018

bqpd commented Mar 28, 2018

galbramc commented Mar 29, 2018

bqpd commented Mar 29, 2018

galbramc commented Mar 29, 2018

bqpd commented Mar 29, 2018 • edited

galbramc commented Mar 29, 2018

bqpd commented Mar 29, 2018

galbramc commented Mar 29, 2018

bqpd commented Mar 29, 2018

mjburton11 commented Mar 29, 2018

bqpd commented Mar 29, 2018

mjburton11 commented Mar 29, 2018

mjburton11 commented Mar 29, 2018

mjburton11 commented Mar 29, 2018

galbramc commented Mar 30, 2018

mjburton11 commented Mar 30, 2018

bqpd commented Mar 30, 2018 via email

galbramc commented Mar 30, 2018

bqpd commented Mar 30, 2018

mjburton11 commented Mar 30, 2018

bqpd commented Mar 31, 2018

mjburton11 commented Apr 2, 2018

mjburton11 commented Apr 2, 2018

mjburton11 commented Apr 2, 2018

bqpd commented Mar 27, 2018 •

edited

bqpd commented Mar 28, 2018 •

edited

bqpd commented Mar 29, 2018 •

edited