ENH: Individual Fins

[MateusStano]

Checklist

• Tests for the changes have been added (if needed)

Description

Individual Fins model have been added:

• Usage can be seen in the documentation files. All equations and modelling are also described in the docs file.

• A lot of structural changes were made to all fin classes. Please review this new structure with care.

• If you run a simulation with the standard Fins model and another with individual fins, the results will be a little different. This happens because the individual fin model calculates forces for each fin separately, using the airflow at each fin’s exact position. The standard model (Barrowman) treats all the fins as one and uses a single point along the rocket’s center. In theory, this means that individual fins are more accurate then the standard model

• There are a couple of TODOs relating to cant angle control. The center of pressure position relative to CDM is precalculated in the Rocket class and then accessed in the Flight. This causes a problem if the fin’s cant angle changes during the simulation because that changes the center of pressure position, but right now there’s no easy way to update these values in the Rocket while the simulation is running.

Breaking change

• Yes
• No

[MyroslavRepin]

Is this branch will be in production soon? Where can I read updated README?

[Gui-FernandesBR]

Is this branch will be in production soon?

Not at all!

Where can I read updated README?

I didn't understand this question

---

If you want to, you can fork this branch and continue the implementation on your fork, thus proposing a PR to our repo later

[MyroslavRepin]

Are you planning adding individual fins configurations, and custom functions into simulation loop?

[Gui-FernandesBR]

closed, but can be re-opened when ready

[MateusStano]

@phmbressan @Gui-FernandesBR this is ready for a new review. I added tests and addressed all the comments in the last few commits

[Copilot]

Pull request overview

Copilot reviewed 42 out of 44 changed files in this pull request and generated 7 comments.

[Gui-FernandesBR]

I will review this PR this weekend

[Gui-FernandesBR]

a few comments left, but overall a very good implementation

My recommendation is to merge this PR already create a new release version

[Gui-FernandesBR]

Suggested change

	Given oposing canards at 0 degrees and 180 degrees, having opposite cant angles,
	Given opposing canards at 0 degrees and 180 degrees, having opposite cant angles,

[Gui-FernandesBR]

Suggested change

	is the reference length, which is equal to the rocket diamete, and
	is the reference length, which is equal to the rocket diameter, and

[Gui-FernandesBR]

TODO for later: the rocketpy/prints/aero_surface_prints.py file could be separated into different, smaller files.

I'd appreciate if you could add this TODO at the top of the file

[Gui-FernandesBR]

rocketpy/rocket/aero_surface/fins/_geometry.py could be divided into smaller parts

I would expect different unit tests, covering each geometry method

[Gui-FernandesBR]

please reference in the code

[Gui-FernandesBR]

I saw the TODO, but it lacks your full explanation. By reading the comment, a young developer could easily become keen to solve a very hard non-profitable problem

[Gui-FernandesBR]

could we fix in list[AeroSurface] only?

        surfaces : list[AeroSurface], AeroSurface

[Gui-FernandesBR]

What is R?

[phmbressan]

In this method (and a few other ones) is the filename being used? Should we use show_or_save plot for them?

[phmbressan]

Generally I don't mind much the abbreviated names. But even though it is clear to most people this is a diameter, this is documented nowhere (I think) and is a bit too short of a name. Perhaps rocket_diameter and make a local alias before big formulas?

[Gui-FernandesBR]

good catch, it's even in our documentatiom to not use single letter attributes

[phmbressan]

Optional, this name is bit too verbose (e.g. I don't think the word run really accomplishes anything here and chain gives a sense of order, but it is vague). I would strive for simplicity with _update_geometry, but it is just a suggestion.

[phmbressan]

This is a bit unusual, since the direction of dependence is reversed here, i.e., a rocketpy more internal object depends on an outer one. If I understood correctly:

• owner will be the Fin (or Fins) themselves. This is odd, since the attribute geometry now depends on the class it is part of. It is almost circular;
• this attribute geometry mutates the outer object, which is also unusual, unless there is an reason for it.

Could you give some hints on why it was not possible to instantiate a geometry with purely a few values from the Fin and Fins classes, but instead, it was necessary to input the full object?

Ideally, the geometry class should not mutate the outer object, but instead have methods and properties (e.g. geometry.Af, geometry.roll_interference_factor etc) that compute and return the values the outer object (i.e. the fin classes) need.

[phmbressan]

Here it says it returns, but the methods returns None.

[phmbressan]

Are these evaluate_ methods kept for backwards compatibility? I don't see why it would be useful to an user, and, hence, they should not be public.

Why not have a new property (or a cached_property or a funcify) that is .center_of_pressure? You can keep these evaluate_ methods for backwards compatibility. Maybe I overlooked some condition on why there should be these evaluates, so please, correct me if I am wrong.

Note: a similar reasoning applies to the evaluates_ of the geometry classes:

• one could probably convert most evaluates into individual properties (e.g. evaluate_center_of_pressure -> .center_of_pressure);
• when a parameter is set in the Fin(s) class, one can either:
  1. recreate the geometry instance with the new value;
  2. have a setter in the geometry that sets the new value and calls the correct re-evaluate for better performance (in this case, the evaluates should be private geometry._evaluate..., since they would only be used internaly).

[phmbressan]

Also, ideally, this method should not be needed, since the update of the parameters would all be done inside the geometry, by a setter. The sequence would similar to:

User (action fin.root_chord=0.5)
    TrapezoidalFin.root_chord setter (sets fin.geometry.root_chord=0.5)
        _TrapezoidalFinGeometry.root_chord setter (evaluates self._root_chord=0.5 and self.update_geometry())

Note:

• the update geometry would mostly delete any caches of the geometrical attributes of the _TrapezoidalFinGeometry (e.g. center_of_pressure));
• The getters of the TrapezoidalFin would generally delegate to the geometry, e.g.:
  - def root_chord -> self.geometry.root_chord

We can discuss more (or go with the current implementation) if this does not seem to tackle our class schema/needs.