-
Notifications
You must be signed in to change notification settings - Fork 0
User/tanmaythakral/67 create fluid force computation class #76
base: main
Are you sure you want to change the base?
User/tanmaythakral/67 create fluid force computation class #76
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work so far! I made a few comments for you to address. Let me know if you have any follow-up questions regarding my feedback.
def calculate_attack_angle(self, apparent_velocity: NDArray, orientation: Scalar) -> Scalar: | ||
"""Calculates the angle of attack formed between the orientation angle of the medium | ||
and the direction of the apparent velocity, bounded between 0 and 180 degrees. | ||
|
||
Args: | ||
apparent_velocity (NDArray): The apparent (relative) velocity between the fluid and | ||
the medium, expressed in meters per second (m/s). | ||
orientation (Scalar): The orientation angle of the medium in degrees. | ||
|
||
Returns: | ||
Scalar: The angle of attack formed between the orientation angle of the medium and | ||
the direction of the apparent velocity, expressed in degrees | ||
and bounded between 0 and 180 degrees. | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function behavior can be slightly improved: the angle of attack should be bounded between [-180, 180). In practice, it is more common to use this symmetric range. Mapping to [0, 180] can cause arbitrary results. For example,
apparent_wind
= [0,0]
orientation
=
Then,
apparent_wind
= [0,0]
orientation
=
Mapping to [0, 180] would also yield
You can use the function boat_simulator.common.utils.bound_to_180
in our utils package to map to a symmetric range.
One more thing as well: whenever you are using Euler Angles, always define the angle convention:
- Where is 0 degrees location?
- Do angles increase clockwise or counterclockwise?
In this case, we use 0 degrees on the positive x-axis and increasing counterclockwise (CCW) for the boat simulator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good!
Will resolve
@@ -37,6 +43,32 @@ def __init__( | |||
self.__areas = areas |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only use a constant area. Switch your implementation to use a single area rather than interpolating areas
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will switch to single area
drag_coefficient = np.interp( | ||
attack_angle, self.__drag_coefficients[:, 0], self.__drag_coefficients[:, 1] | ||
) | ||
area = np.interp(attack_angle, self.__areas[:, 0], self.__areas[:, 1]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per my previous comment, let's use a constant area for our implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will switch to single area.
# Calculate the lift and drag forces | ||
lift_force_magnitude = ( | ||
0.5 * self.__fluid_density * lift_coefficient * area * (velocity_magnitude**2) | ||
) | ||
|
||
drag_force_magnitude = ( | ||
0.5 * self.__fluid_density * drag_coefficient * area * (velocity_magnitude**2) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These can probably be abstracted into a separate function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good!
# Rotate the lift and drag forces by 90 degrees to obtain the lift and drag forces | ||
# Rotate counter clockwise in 1st and 3rd quadrant, and clockwise in 2nd and 4th quadrant | ||
# 0 otherwise | ||
drag_force_direction = (apparent_velocity) / velocity_magnitude |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I recall correctly, you are only rotating the lift by 90 degrees. To be clear, I understand that you do indeed rotate the drag as an intermediate step, but lift is rotated by 90 degrees relative to drag.
I might also rename this variable to denote this as the drag unit vector
print(lift_force, drag_force) | ||
print(calculate_magnitude(lift_force), calculate_magnitude(drag_force)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you're finished debugging, any print statements should be removed from test functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do
def test_calculate_attack_angle( | ||
apparent_velocity, orientation, expected_angle, medium_force_setup | ||
): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great test suite so far. A few suggestions for some more tests:
- Try using orientations that fall outside of the typical expected range (negative numbers, larger than 360, etc.)
def test_compute_forces( | ||
medium_force_setup, orientation, expected_lift, expected_drag, apparent_velocity | ||
): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions on additional tests:
- Testing with an apparent velocity with a magnitude other than 44
- Testing with an orientation other than 0
- Try testing with orientations that might lie outside the expected range (negative, larger than 360, etc.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The NASA simulator only has values for velocity 44m/s. Will need to manually develop tests in this case.
Will do but the NASA sim only provides me with magnitude so it'll still have the same magnitudes in these cases. (Cant test direction)
area = np.interp(attack_angle, self.__areas[:, 0], self.__areas[:, 1]) | ||
return lift_coefficient, drag_coefficient, area | ||
|
||
def draw_boat(ax, position, orientation): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this function belong in this class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where should it go?
def visualize_forces( | ||
self, apparent_velocity, lift_force, drag_force, position=[0, 0], orientation=0 | ||
): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this function belong in this class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where should it go?
Description
Verification