Open mayork opened 7 years ago
Awesome, thank you.
sweet, looking forward to seeing how this goes!
alright already got climb removed. was easier than I anticipated.
@pgkirsch looks like one enhancement is going to be discretizing the breguet range (it's going to require extra work to remove that feature, i dont see any point in doing that)
Fine by me to have a discretized (more accurate) breguet range.
@pgkirsch @1ozturkbe I think this is done. What we have now is Philippe's paper plus the all the updates that we discussed (since I just removed from our current code base). The updates I believe this includes are:
Philippe note that I believe this changes a few of the HT sizing constraints (you should check this...I made the change because of some complications in flight profile integration, namely needing to include a CG)...it's described in the attached write up. Let me know your thoughts about this. Basic_HT_Sizing_Model (3).pdf
In the bulleted list above I noted in brackets who should update the manuscript to reflect each new model. @1ozturkbe and @pgkirsch thoughts on that? Philippe you tell us how you want us making manuscript updates.
Berk - if you could VSP the airplane this outputs that would be good for debugging anything I messed up.
Philippe feel free to posts questions once you look over the code and see how the solution compares to what is currently in the paper (update engine subs first, noted in a commit comment)
@whoburg and the other who advocated for working from current code I think that was the right call. This only took me ~3 hours (could be some bugs still, needs VSP and solution comparison).
@pgkirsch if you update the engine subs and post a solution comparison to what you had in the paper manuscript I can take a look at any issues tomorrow.
@pgkirsch forgot to mention that to run the model import the optimal 737 class from the pk_SP_aircraft.py file. All the aircraft level constraints are again still in D8.py
@mayork You're a machine, thanks for doing all of this. I'm going to spend some time getting my bearings and figuring out what's what.
Does it usually take a ridiculously long time to build the model? If so, why is that?
@pgkirsch you probably need to pull master. It is a much larger problem then your baseline models due to the vectorization but should solve in a few seconds (3.7 seconds for me on verbosity=4)
For manuscript editing, I think we should PR to the pkjournal branch of the pubs repo. Give me a chance to understand better how much has changed from my original stuff though before we do that. Btw @mayork what happened in the last few commits to the pkjournal branch? You seem to have made some commits, reverted them, and also reverted one of my commits?
@pgkirsch I removed statelinking which I thought we didn't need but i'm pretty sure we do so I ended up reverting back to that. I lost some changes I made to to the substitution file in the process of some local experimental reverts so I ended up just repushing it all back to the branch.
Which one of your commits did I revert? All I see is commits I made.
I think PR on the pkjournal branch of pubs is a good idea. @1ozturkbe is that good?
One general question I have about the current code structure, is how should I interpret the module naming? I think we previously had aircraft.py
, fuselage.py
, vtail.py
, wing.py
etc. and now we have D8.py
, D8_Fuselage.py
, D8_VT_yaw_rate_and_EO_simple_profile.py
, D8_Wing_simple_profile.py
etc. Do these names tell me something I need to know? Why did they become so long and descriptive? If everything is for a D8, why are they all called D8_something? If everything can actually be applied to a conventional-config aircraft, why are they all called D8_something?
@mayork @1ozturkbe @whoburg
@pgkirsch treat D8.py
as aircraft.py
, D8_Wing_simple_profile.py
as wing.py
, etc. The D8 tags on everything is a hold over from a previous naming convention that we need to change. Another change I made is that I integrated all of the sub components like wing and HT etc. with a simple aircraft and flight profile model for testing purposes, hence the tag simple_profile
on the files we are importing from. We used to have a two VT files hence the additional VT_yaw_rate_and_EO_
there, but it's nothing important. Just an identifier Berk and I used that needs to be changed.
@pgkirsch responses are above beneath your comments.
@pgkirsch @1ozturkbe I updated the sub values and I think the solution is looking pretty decent. It's a bit more accurate to the reference aircraft than what was in your original manuscript.
@1ozturkbe you should generate a VSP of the aircraft...could be good for the paper
@mayork super time + internet limited atm, but will do my best.
@mayork Thanks for updating the sub values and checking the solution.
Solving the model is indeed relatively quick, but actually just building it (i.e. importing the method) takes > 20 seconds. Is that typical?
Sorry I was referring to the pkjournal branch of the pubs repo (you pushed some commits to that branch it seems).
Ok, understood on the naming. It seems like from a reducing-code-duplication point of view, it would make sense to import the simple profile part for each of those models. I am happy to go through and change names if that is useful forward work? Although I guess it isn't a priority at the moment.
A VSP would be great! That's such a cool new feature.
@pgkirsch yeah i somehow was on that branch and not master and pushed my engine paper to it then reverted and pushed the engine paper to master...i didn't think that i overwrote any of your commits. Sorry if I did!
It takes under a second to perform the import on my computer.
Berk and I can handle the naming. Parts of the simple profile should be imported but it is difficult to import a lot because all the simple subsystem models change depending on which detailed model is used due to how the coupling between the detailed subsystem models works.
Gotcha, makes sense.
And as for pubs, it's commits 449291d and b56838 that I'm most confused about. The former is a merge that I'm not sure I understand, and the latter is reverting a commit I made.
Unless that merge was particularly important, I'm going to create a new branch starting from when I last edited that branch, and that is the branch from which we should work @mayork @1ozturkbe.
@mayork @1ozturkbe @whoburg New branch on pubs for this work is called spajoa
(SP Aircraft Journal of Aircraft). Please work on this branch for this paper from now on. I just pushed to origin, so you should be able to pull and checkout.
all the commits i made should not have been there (it took me a while before I realized I wasn't on master...)...anyways, working on the new branch sounds good.
@mayork, a few questions about the vertical tail model:
I don't seem to be able to find the first two vertical tail model constraints (constraints (14) and (15) in the current draft), although the variables still seem to be declared. Did you just take that sizing case out altogether? Same for constraint (18), presumably for the same reason.
MY: Constraint 14 is on line 228 of D8.py
. Constraint 18 is on line 489 of D8.py
. It looks like constraint 15, which defined l_{vt} has been replaced by the constraint on line 502 of D8.py
. @1ozturkbe do you remember why we did this? I'm pretty sure it has to do with the coordinate frame CG model. It was important to choose the coordinates carefully and when we did it simplified a lot of stuff.
PK: OK, gotcha. These are multi-component constraints so they live in a system-level model (for what it's worth @bqpd and I don't see eye-to-eye on this, but I will save that discussion for another time). For (14), it now has numVT
. Where does this get set to 1? Sounds like there is an action item for @pgkirsch / @mayork / @1ozturkbe to update constraint 15 in the manuscript.
''n_{VT}"
is set to 1 on line 112 of subs_optimal_737.py
2) Nomenclature question: why is the engine area called A2? Is that a TASOPT convention? Related: I would like to avoid re-writing constraints in the manuscript with new nomenclature, so insomuch as possible, I would like to adhere to current nomenclature when adding new constraints. I don't think that we should start from scratch with .latex()
because it's still fairly manual and I put a fair amount of time getting constraints to where they are. Action for @pgkirsch: update manuscript to remove constraint (19).
MY: A2 is TASOPT convention, the variable name is a carry over from my engine model. We generally kept your nomenclature, I don't remember renaming any of your variables at any point.
PK: Cool, good to know. A2 vs. Afan seems to be one of the few exceptions I've found so far, others being the Engine Out (EO) suffix added in a couple places.
3) Constraint (21) (dxtrail >= croot + dxlead
) seems to be commented out. How come?
MY: I believe it was inactive and we deemed it unnecessary? You'll also note that all the variables in that constraint are commented out. Berk spent a lot of time on the CG model and was able to remove some things that became unnecessary after he made it. @1ozturkbe does that sound familiar?
PK: Hmm OK, but I don't think the point of using GP/SP for aircraft design is to eliminate constraints that aren't active. It might become active in some weird corner of the design space, after all. Also, I'm surprised it wasn't active. I would be a proponent of uncommenting it, if you guys are alright with it. (possible action item @pgkirsch)
MY: I thought about this more and realized I think that the constraint is accounted for by the one on line 486 of D8.py
. @1ozturkbe and @pgkirsch fact check that, could be fake news. For the record I agree with what you were saying about inactive constraints.
BO: I removed everything about trailing and leading edges because it was over-defining the problem, and causing it to fail. I pretty much made the assumption that the C.G was going to be at midchord, and removed a bunch of aux. variables. We always end up getting the same result as expected anyways, with the t.e. being right at the end of the fuselage. This also drastically reduced the number of GP solves. This applies to comment 4 as well.
4) Constraint (22) seems to have been modified from lfuse >= xCGvt + dxtrail
to be lfuse >= xCGvt
. How come?
MY: Supplanted by the constraint on line 486 of D8.py
. I checked the solution and these variables make sense (lfuse=36.6m while xCGVT is 33.5m)
PK: Got it
5) Constraint (24) (mean aero chord) appears to have been changed to a signomial equality constraint. I'm a bit surprised it was necessary, but it's cool. It does however raise the issue that we need to introduce that we use signomial equality constraints in this paper. Action for @pgkirsch: Add something to the introduction explaining this.
MY: feel free to more or less reuse the top of page 6 in my engine paper (http://hoburg.mit.edu/publications/turbofanSP.pdf).
PK: Cool, thanks!
BO: mac is a SigEq now because the constraint was untight. It turns out that it is not clear which direction the pressure on the variable is in. Doesn't end up increasing the runtime in any case. But if this is problematic you could try to relax it once again and see what happens.
Action item for @pgkirsch or @mayork to update to the correct drag fit for constraint (31) (and to only use one equation number).
6) Surely, a version of the WingBox class does not belong in each flight surface model?! I'm pretty sure I previously had a wingbox.py. If necessary to modify it for different surfaces that should be done by passing arguments to the class, and using conditionals to add/modify constraints/substitutions, as you did with the isinstance
block. Thoughts on this @whoburg? If agreed, I would suggest, either an action item for @mayork @pgkirsch.
MY: The plan was for that to be the case, I think this was just a low priority and was never done in the time crunch. Just created an issue to document this (#96). Before this is done I think @1ozturkbe should be consulted to ensure everything works right with the updated wing and HT structural models.
PK: Fair enough. Please take the anal code structure comments with a grain of salt.
BO: Agree with everything Martin said.
7) Seems like the upper bound on max thickness was changed from 0.15 to 0.14. @pgkirsch: update manuscript to reflect this.
MY: Yes, that aligns better with TASOPT but can be changed back if you to 0.15 if you wish.
PK: Nah, that's fine with me
8) Action for @1ozturkbe to update constraint (40) in the manuscript with the new nu, lambda fit. (Was it really a marked improvement?)
9) Constraint (43) seems to have changed to use numspar
. (a) Would you mind explaining the new constraint a bit?, and (b) Are you aware it seems to be an unbounded free variable?
MY: I just checked and 'N_{spar}' is 1 in the solution (it's set on line 109 of subs_optimal_737.py
). This was used for modeling structurally reinforced VTs on the D8 project. It should be set 1 for what we're doing now.
PK: OK. I see now that the fVT
bit that I couldn't find earlier appears at line 264. Presumably CVT is set to 1 in the subs...
file too? Anyway, more importantly, (43) can be left unchanged in the manuscript right? (albeit with a cleanup, possibly (@pgkirsch)).
10) Not sure I can see where constraint (44) (max tail force) went.
MY: It's now on line 214 of D8.py
.
PK: OK
General comment: constraints that link submodels (i.e. where a constraint has a variable that lives in two different submodels) it was moved up to live in either aircraft or aircraftP (very occasionally mission). Some of the HT and wing constraints now live in similar place in D8.py
...I found them by controlling f in D8.py
. This constraint placement was really motivated by software engineering factors and prevent the submodels from having a chain of circular imports.
PK: OK. As I alluded to earlier, I don't really like this way of doing things, for the exact reason of it being difficult to find all of the constraints that govern vertical tail design (some of the most critical vertical tail constraints aren't in the vertical tail model). I think what @bqpd helped implement for the Hyperloop One code using .subinplace()
is a more elegant, readable, intuitive solution that is still software-engineering-friendly, but that's a fight for another day. @whoburg
MY: I'm not too familiar with what your hyperloop code looks like but i'd venture a guess that it is in fact a better way to do it.
BO: I like the principle, but I also like the fact that we can control the entire configuration of the aircraft from one file, since all of the coupling constraints between the submodels are one level higher in the hierarchy. In my opinion, it would be inconvenient have to dig into the submodels to modify constraints for different vehicles. But I do see the merits of .subinplace().
@pgkirsch responses are above
@mayork Thanks for the quick response. Responses to your responses also above.
@pgkirsch a few final replies above
Reply above, Kellyanne (@mayork).
Made all of the improvements to vertical tail chapter that aren't waiting on responses.
@whoburg if we get rid of the standalone tests (i.e running vtail by itself) how do you recommend we handle the fixed variable table in each chapter, given that many of these fixed variables are only fixed in the standalone case?
I've also added comments. Sorry about the tardiness.
A general note is that when we say a constraint want tight and bad tk become a signomkal equality the issue generally didn't come up until we integrated with a full flight profile
No worries @1ozturkbe. Thanks for the responses and good catch on number 8! Not sure how that happened...
@mayork if you have bandwidth, I can think of a couple things that would be helpful:
Feel free to pick whichever of these appeals to you most
Alright I'm finishing up some edits to the engine paper (just got reviews on that too), then I'll take a look.
Side note - we never integrated the landing gear....I'm guessing that needs to be done?
@pgkirsch
I didn't even notice. Has the D8 model never had a landing gear model? Would you mind integrating it if you get a chance?
it never had a landing gear because TASOPT never had a landing gear. I can take a stab at integration either tomorrow or over the weekend.
Landing Gear Differences
@pgkirsch @1ozturkbe I got the landing gear integrated. results look pretty good.
Awesome, thanks!
@pgkirsch where do you want to put the moment of inertia and CG models? independent sections at the very begining?
@mayork I think that stuff belongs in the full system chapter, right? There should already be a CG model in there that you can modify/replace.
@mayork @1ozturkbe why is there both a D8_HT_simple_profile.py
and a HT_Simple_Profile_Performance.py
and which one of these can we delete? More generally, if I want to go on a code cleanup rampage, would today be an ok time to do that?
Sorry, should have checked before posting. Seems like we can delete "HT_..." given that it isn't used anywhere?
Similar to my first question, can Wing_simple_performance.py
get nuked?
@mayork running the latest version of this model there seem to be a number of TightConstraintSets returning fairly significant errors (10%, 30%, ~100%). This seems like it could be an issue unless those constraints don't need to be tight?
Seems like Wing_simple_performance.py
is only used by simple_ac_imports.py
and make_figure.py
but nothing uses either of these things. Does that mean all three files are safe to delete?
@pgkirsch yes you can code clean up rampage....I've checked the TCS warnings, they're all okay. Most of them are things aren't necessarily expected to be tight but we sometimes look at how tight/untight they are.
For example, one such constraint is the min static margin requirement which isn't tight at every flight segment
I don't think anything needs to be done about them. If it bothers you a lot I can prob remove the TCS on the relevant constraints
Ok thanks. Happy to leave the TCS's for now, just wanted to make sure the slack constraints weren't signaling a bug. I won't go too crazy, I just want to remove things like wingbox duplication if it's not too difficult, and rename files.
Btw what does relaxed_constants()
do?
@pgkirsch @1ozturkbe I'm starting my attempt at pulling apart the full model to an updated PK journal model. Will keep you posted on progress.