Rename some modules

[ewu63]

Description of feature

The names in pygeo are terrible. There's a module called pygeo inside pygeo which only does surface generation, and DVCon isn't really for constraining DVs directly.

Here is a list of things I would like to rename, together with suggestions.

• DVGeometry -> add FFD to the name to make it explicit
• DVGeometryMulti -> something MultiFFD
• rename DVGeometry prefix for the other parameterizations
• DVConstraints -> geoConstraints
• pyGeo the module must be renamed

Few other things to consider:

• These changes are large enough that I think we should consider a major version bump
• Maybe move the 3 files (pyGeo, pyNetwork, pyBlock) to their own module

Please comment below with your suggestions.

[marcomangano]

I agree with your points, but I am not sure if this is worth a major bump. Unless we actually rewrite a substantial part of the code or rearrange a lot of files, this would "just" be some class renaming. I think we could still handle that with a minor bump and some deprecation warnings until the following minor bump

[gawng]

This is going to break a lot of people's optimization cases that use the old names though. To me, that's enough to justify a major version bump Link.

Regarding DVConstraints rename, what about dvgeoConstraints? The constraints restrict geometric design variables. Or is it that the name would be too long?

[A-CGray]

DVConstraints -> GeoConstraints
DVGeo -> GeoParam, GeoParamFFD, GeoParamFFDMulti, GeoParamVSP, ... etc

[ewu63]

Regarding the major bump, it's just to stop people from complaining about it breaking people's code as I'm sure it will impact everyone downstream. People should never blindly pull/install stuff without reading changelog, but in reality that's what they do.

EDIT: I think it's also possible to still keep the old names but throw warnings when they are used. This can be done by subclassing the renamed modules, and throwing the warning at __init__ before calling super().

Regarding DVConstraints rename, what about dvgeoConstraints? The constraints restrict geometric design variables. Or is it that the name would be too long?

Well all constraints will restrict DVs, so I don't think that's a good argument.

[eytanadler]

DVConstraints -> GeoConstraints DVGeo -> GeoParam, GeoParam_FFD, GeoParam_FFDMulti, GeoParam_VSP, ... etc

I like this suggestion, though without the underscores in the new DVGeo names

[sseraj]

My suggestions:
pyNetwork -> curveUtils
pyGeo -> surfaceUtils
pyBlock -> volumeUtils

The three above appear related enough to put in their own submodule. However, pyGeo is primarily used for surface generation, whereas pyNetwork and pyBlock are used for FFD parameterization.

DVConstraints -> GeoConstraints

DVGeometry -> FFDGeometry
DVGeometryCST -> CSTGeometry
DVGeometryVSP -> VSPGeometry
DVGeometryESP -> ESPGeometry
DVGeometryMulti -> MultiGeometry

(DVGeometryMulti currently only works with FFDs but could be extended in the future.)

I have no strong opinions on the version number. We have broken the API before without bumping the major version.

[A-CGray]

I should have mentioned in my post, but the motivation for having GeoParam at the start of each parameterization class name is that it is then clear that they all inherit from the same GeoParam base class and they will all be grouped together when you sort things in alphabetical order

[ewu63]

Okay I think we have agreed on DVConstraints -> GeoConstraints. I guess we are using CamelCase for class names?

As for the parameterizations, the two proposals currently are:

• GeoParamXXX
• XXXGeometry

I prefer the former just to maintain some consistency with GeoConstraints, i.e. have the Geo prefix. I am also fine with leaving Multi without adding FFD if we make it clear that there is potential in the future to implement other multi-geometries.

Finally, for the topology stuff, I prefer not naming a class containing the word Utils. Do we ever use pyNetwork outside of the context of RefAxis? What about pyBlock and FFD? What I mean is, can we do something like the following without loss of generality:

• pyNetwork -> geoRefAxis
• pyBlock -> geoFFD

or something like that. We could also do the classic py- prefix if we really want. As for pyGeo, something like SurfaceEngine could work. Again, this implies certain functions on the somewhat general classes, but I don't see any problems with that.

One final point of discussion: should we merge pySpline into this? The way I see it, pyNetwork, pyGeo, and pyBlock are just "advanced" wrappers on the corresponding pySpline objects, where with the topology stuff you can have multiple of them together. So it would make sense to merge them in some way. Do we have any examples of codes that use pySpline without needing pygeo?

[sseraj]

I'm fine with the GeoParam prefix and @nwu63's proposed names for pyNetwork, pyGeo, and pyBlock.

[A-CGray]

It looks like both pyNetwork and pyBlock are basically collections of lines/volumes, so why not call them curveCollection and volumeCollection?

pyGeo could be called surfaceCollection if it basically does the same thing as pyNetwork and pyBlock for surfaces. Or, if it has much more functionality, call it surfaceEngine given that that's what the docstring says it is

[ewu63]

pyGeo is primarily (I think AFAIK only) used for generating wing surfaces. I kind of prefer SurfaceEngine but then we lose the nice consistency with XCollection, so I'm a bit torn.

[eirikurj]

Sorry for the delay on this. Here are my comments.

I guess we are using CamelCase for class names?

Yes, we should use CamelCase.

Should we merge pySpline into this? Do we have any examples of codes that use pySpline without needing pygeo?

I see the appeal of merging, but I would be hesitant to do it. pySpline is a more fundamental package, its well-defined, self-contained, and I think there are many applications beyond just geometry where you would want to use splines without having to install the extra “baggage” of pyGeo. Internally, I think prefoil and weightandbalance use it without pyGeo and most likely something else that I am forgetting. I use it for data interpolation, which is not geometry. Also, based on external activity and questions and issues we have got on GH I suspect that people are using it without pyGeo. So in short, I think we should keep it separate.

Do we ever use pyNetwork outside of the context of RefAxis? What about pyBlock and FFD?

I think even if we use pyNetwork primarily for a reference axis, it is (or should be) a general component and implementation, that does not necessarily need to be a reference axis. The same goes for pyBlock, its not necessarily only an FFD. In some sense, refAxis could be subclass of pyNetwork, and FFD could be a subclass of pyBlock.

I feel pyGeo is a little special since it has more features beyond the other two. It supports generating lifting surfaces from more elemental description (points, rotations, sections etc.), which is probably why we have different suggestions on renaming. It should probably be split into two or more classes, a more general class without this specialization, and then extra functionalities such as lifting surfaces. Alternatively, the lifting surface generation could be a helper function, returning a pyGeo object.
The name pyGeo is maybe not the best and is probably too general for what it is. In some sense pyGeo is more fitting to describe an object that is a combination of all geometry objects (curves, surface, etc.). We could consider other names, like XCollection, for the similar functionality as found in pyNetwork and pyBlock, and something else for the lifting surfaces.

I feel that these names, while they are not perfect, are somewhat descriptive for what they are and how we use them. Its possible that I am just too used to them also. I think if we are renaming these three classes, then XCollection could be appropriate for parts of the functionality found in these classes, but then we should use the opportunity to refactor these classes also.

As for the parameterizations, the two proposals currently are:
GeoParamXXX
XXXGeometry

Overall, I think GeoParamXXX and GeoConstraintXXX are probably fine. DVCon is more of a misnomer than DVGeo in my opinion, but its probably good to change both for consistency if we plan to rename either.
However, renaming DVGeo will have some impact as this name is used in all of our solvers, aside from scripts. Its easy enough to change these, but I just want to make sure that we are changing for a good reason.

In any case, we should probably discuss this a little further, and converge on names and features, but I think much of the pyGeo repo could be refactored as part of the process. I am willing to do the refactor and help as needed.

[joanibal]

Here is my 2 cents on this issue as well.

People should never blindly pull/install stuff without reading changelog, but in reality that's what they do.

I think it is a bit more nuanced than that. The docker images are based on the latest versions so changes just appear there without a user directly pulling from pyGeo. So scripts that need to work with the latest docker images will break. This is certainly something that has happened to me before.
Additionally new students in the lab may be given a desktop setup with the new pyGeo but scripts that use the old API.
This feels like it very obviously should be a major version bump to me. This is going to break things for people, and it might not be because they are flippant about managing their libraries.

API changes like this are a bit of a pain point for me. I agree that the new names are clearer, but the majority of the people who use pyGeo are already familiar with the current API. Changing it on them now will be confusing in a different way. Additionally, most of our optimization scripts will also need to be updated. This API change will also detract from other work such as a proper refactor of local DVs or child FFDs.
I think it is worth discussion the costs of switching to the new API. Maybe it is worth waiting until "the big refactor" or adding this change to a dev branch where we accumulate other big changes like this.

[ewu63]

The docker images are based on the latest versions so changes just appear there without a user directly pulling from pyGeo. So scripts that need to work with the latest docker images will break. This is certainly something that has happened to me before.

We should be publishing tagged production releases of the docker images, rather than having users rely on a rolling release.

Additionally new students in the lab may be given a desktop setup with the new pyGeo but scripts that use the old API. This feels like it very obviously should be a major version bump to me. This is going to break things for people, and it might not be because they are flippant about managing their libraries.

API changes like this are a bit of a pain point for me. I agree that the new names are clearer, but the majority of the people who use pyGeo are already familiar with the current API. Changing it on them now will be confusing in a different way. Additionally, most of our optimization scripts will also need to be updated. This API change will also detract from other work such as a proper refactor of local DVs or child FFDs. I think it is worth discussion the costs of switching to the new API. Maybe it is worth waiting until "the big refactor" or adding this change to a dev branch where we accumulate other big changes like this.

All fair points. We also discussed this in the maintenance meeting, and agreed to keep the naming as is for now due to a number of reasons:

• this is a major backwards-breaking change
• some names are still undecided
• some refactor is in order

I think we should keep this issue open though, and have some sort of long term plan to ensure pygeo is still maintainable in the future.