search

OpenMDAO / Aviary

NASA's aircraft analysis, design, and optimization tool
https://openmdao.github.io/Aviary/
Other
107 stars 51 forks source link

Rename run_aviary #304

Closed xjjiang closed 1 month ago

xjjiang commented 1 month ago

Summary

Rename run_aviary to setup_and_run_aviary because that is what it does.

Note: it will cause backward incompatibility. Should we make this change?

Related Issues

Backwards incompatibilities

updating the api will cause backwards incompatibility for any users currently importing run_aviary from the api

New Dependencies

None

crecine commented 1 month ago

This might be a good opportunity to add a deprecation warning to the API, and support both for now.

xjjiang commented 1 month ago

This might be a good opportunity to add a deprecation warning to the API, and support both for now.

It would be a good practice for the future. Any guideline? @crecine @Kenneth-T-Moore

crecine commented 1 month ago

@Kenneth-T-Moore I tried adding a deprecation warning, but running examples/run_basic_aviary_example.py doesn't actually produce a warning. https://github.com/xjjiang/om-Aviary/pull/1/files

johnjasa commented 1 month ago

I'm not certain if this change is a helpful one for users and I want to discuss it here.

Although the run_aviary command does indeed setup and run the model, I think the name setup_and_run_aviary is clunky, especially for low-level uses of Aviary. I think run_aviary is much more memorable and clear.

@xjjiang could you tell me more about the benefits of renaming here? I welcome input from others too!

Kenneth-T-Moore commented 1 month ago

@crecine
I am not sure what is swallowing that warning. I would have to play around.

@johnjasa I am not sure we need this change either. I think the level 1 user should stay unaware of the setup/run distinction until they have learned the basics.

crecine commented 1 month ago

I believe this was intended to reduce some confusion regarding the difference between run_aviary and run_aviary_problem. Are we expecting level 1 users to be importing from the api? I would think that this would have the biggest effect on level 2 users that want to script/automate running Aviary

xjjiang commented 1 month ago

I just thought setup_and_run_aviary is what this function actually does. It is okay if we keep as it is.

xjjiang commented 1 month ago

I will withdraw this PR in one hour if you do not appose it.