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

User Interface for Modeling Aircraft #462

Open
wants to merge 28 commits into
base: main
Choose a base branch
from

Conversation

Dawson-Manning
Copy link
Contributor

Summary

This commit adds a interface for users to easily create and edit models to use for optimization. Interface includes names, units, and descriptions of each variable and sorts them into four tabs for Aircraft, Dynamic, Mission, Setting and sorts each variable name into containers based on the subheading "wing", "engine", etc. Additionally, there is an added search feature which allows for users to quicky search a variable name to be edited. Once finished with entering inputs, the user's inputs and remaining entries, which autofill to default values, are written to a csv file in a format that can be used by Aviary. Image of interface attached below.
Screenshot 2024-07-26 155255

All information is pulled from Aviary>aviary>variable_info>variable_meta_data.py and Aviary>aviary>variable_info>variables.py thus it automatically updates and adds variables to the interface when variables are added to Aviary's meta data.

Additional edits were made to Aviary>aviary>variable_info>variables.py to keep formatting consistent with other variables.

Related Issues

  • Resolves #

Backwards incompatibilities

None

New Dependencies

None

Copy link
Member

@johnjasa johnjasa left a comment

Choose a reason for hiding this comment

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

Thanks for this, Dawson! I tried it out locally and it worked first try. I like the collapsible headings for each subsystem.

I added some small comments and I'll put some general ones here:

  • could you please add a command mapping for this utility so that someone could do aviary build_model (or whatever name you choose) in the command line, like the other utilities Aviary has? There's a little bit of convoluted code to do this, but you can follow the other utilities' logic here or I'd be happy to walk through this process with you
  • when you hit Submit could you make some sort of indication that a file was saved? I hit submit and initially thought it didn't work because I didn't see a confirmation or similar message, though a file was successfully created. This notification could be a message in the command-line window, a pop-up in the GUI, or something else. I think it'd be useful to let the user know that a file was made explicitly

Feel free to disregard any of these suggestions, I just wanted to weigh in. Thanks for the nice work!

Comment on lines 3 to 7
from tkinter import *
import tkinter as tk
import tkinter.ttk as ttk
from tkinter import messagebox
from tkinter import filedialog
Copy link
Member

Choose a reason for hiding this comment

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

Some of these imports are unnecessary; with the from tkinter import * all parts of tkinter are imported. You could keep either that line (not too common to do) or explicitly import the specific things you need from tkinter.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the cleanest is just to have the 2 aliased imports (tk, ttk), and use those to call all needed functions. The two functions specifically imported here don't seem to be used more frequently than other tkinter functions, so to me there isn't a clear reason to specially import those.

I'll weigh in that we shouldn't globally import here. That's something we should only do if there is a very good reason to specifically do that.

else:
entries_per_subhead.append(1)
name_each_subhead.append(key)
headers.append('dynamic')
Copy link
Member

Choose a reason for hiding this comment

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

Dynamic variables (all those that are Dynamic.Mission.<variable_name> are not actually used in the .csv file. Because of this, I don't think they need to be included in this GUI.

There could be value in having the dynamic variables present in the GUI and you then output those values as initial guesses used in the phase_info object, but I think that capability is already represented by the draw_mission tool. So, I'd suggest removing the dynamic variables from this GUI unless I'm missing a reason (which I easily could be!)

@Dawson-Manning
Copy link
Contributor Author

With new commits, the gui can now be ran using aviary command . Additionally, all Dynamic.Mission.**vars were removed along with the associated tab. The gui will now indicate to the user via command prompt when a file has been saved with "Summit", "File->Save", and "File->Save as...", The gui will now indicate to the user via command prompt when a file has been opened with "File->Open" and "File->Open & Display".

@@ -97,10 +100,11 @@ def aviary_cmd():
cmdargs = [a for a in sys.argv[1:] if a not in ('-h',)]

if len(args) == 1 and len(user_args) == 0:
if args[0] != 'draw_mission' and args[0] != 'run_mission':
if args[0] != 'draw_mission' and args[0] != 'run_mission' and args[0] != 'build_model':
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if args[0] != 'draw_mission' and args[0] != 'run_mission' and args[0] != 'build_model':
if args[0] not in ('draw_mission', 'run_mission', 'build_model'):

@@ -155,7 +155,7 @@ class Design:
FINENESS = 'aircraft:design:fineness'
FIXED_EQUIPMENT_MASS = 'aircraft:design:fixed_equipment_mass'
FIXED_USEFUL_LOAD = 'aircraft:design:fixed_useful_load'
IJEFF = 'ijeff'
IJEFF = 'aircraft:design:ijeff'
Copy link
Contributor

@crecine crecine Aug 6, 2024

Choose a reason for hiding this comment

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

This one is intentional (and does not need to be set by the GUI)

Suggested change
IJEFF = 'aircraft:design:ijeff'
IJEFF = 'ijeff'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@crecine
Without this modification "ieff" becomes a subheading and since this variable is in the middle of the "design" section it takes the rest of the design variables into its container. Would it be better to move this variable to later in the variable list or is there a better option?

Copy link
Contributor

Choose a reason for hiding this comment

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

if we need this change to prevent that issue, that's fine. But since we don't need to set this value, excluding it from the GUI would also work

@crecine
Copy link
Contributor

crecine commented Aug 6, 2024

There are some variables that have different defaults for GASP vs FLOPS (see utils/legacy_code_data). It might be a good idea to allow the user to load the defaults from one of those before they start making changes.

@Dawson-Manning
Copy link
Contributor Author

There are some variables that have different defaults for GASP vs FLOPS (see utils/legacy_code_data). It might be a good idea to allow the user to load the defaults from one of those before they start making changes.
@crecine
I am able to do this, however since there are far more default values for flops the resulting csv file would be a mixture of FLOPs and GASP default values as this interface prints all lines from the variable meta data except dynamic variables. Is this an issue???

@jkirk5 jkirk5 self-requested a review August 7, 2024 20:26
@crecine
Copy link
Contributor

crecine commented Aug 15, 2024

@crecine
I am able to do this, however since there are far more default values for flops the resulting csv file would be a mixture of FLOPs and GASP default values as this interface prints all lines from the variable meta data except dynamic variables. Is this an issue???

I meant adding a button (or something else) to allow the user to load one of the legacy defaults after it reads the variable_meta_data

Copy link
Contributor

Choose a reason for hiding this comment

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

This file needs docstring.

@hschilling
Copy link
Member

Thanks, Dawson!

In the search tab, if I put something in the search box, do a search, and then press Clear, the results go away as they should but the search box still has what I typed into it. Should it? I think most search interfaces clear the search text box also.

@hschilling
Copy link
Member

When you do a Submit, the text in the terminal has a typo

Model successfully Seved to Aircraft_Model.csv

@hschilling
Copy link
Member

This field looks like it might be hard to fill out. Can a popup menu be used for values that have a limited number of options?

image

@hschilling
Copy link
Member

Not sure what is different from when I tried the Search and commented about it, but now when I do searches, nothing shows up. For example, searching for "wing" results in no results.

image

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.

6 participants