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

Station keeping practice worlds #582

Merged
merged 10 commits into from
Apr 8, 2023
Merged

Station keeping practice worlds #582

merged 10 commits into from
Apr 8, 2023

Conversation

caguero
Copy link
Contributor

@caguero caguero commented Feb 7, 2023

How to test it?

Launch the simulation:

ros2 launch vrx_gz competition.launch.py world:=practice_2022_stationkeeping0_task

Launch the teleop node:

ros2 launch vrx_gz usv_joy_teleop.py

Now, you can move around and check the goal and errors by echoing the topics:

/vrx/stationkeeping/goal
/vrx/stationkeeping/mean_pose_error
/vrx/stationkeeping/pose_error

Signed-off-by: Carlos Agüero <[email protected]>
@M1chaelM
Copy link
Collaborator

See #596 . The worlds seem to be working fine, but the visualization markers are currently missing.

M1chaelM
M1chaelM previously approved these changes Feb 17, 2023
@M1chaelM
Copy link
Collaborator

@caguero I approved this, but I'm on the fence as to whether we should first fix the wayfinding markers before merging, since it's so much easier to test the worlds with them on.

@caguero
Copy link
Contributor Author

caguero commented Feb 17, 2023

@caguero I approved this, but I'm on the fence as to whether we should first fix the wayfinding markers before merging, since it's so much easier to test the worlds with them on.

I can certainly take a look at how hard would be to port the markers.

@caguero
Copy link
Contributor Author

caguero commented Feb 20, 2023

ros2 launch vrx_gz competition.launch.py world:=practice_2022_stationkeeping0_task

See #597

Signed-off-by: Carlos Agüero <[email protected]>
@M1chaelM
Copy link
Collaborator

M1chaelM commented Feb 21, 2023

This was much easier to test with markers working. As I went through the practice worlds again I noticed that the environmental conditions don't seem to vary too much. Looking at the files confirms the wind and waves are identical. I can't remember if this was the case in VRX classic. Do we need to represent a range of conditions across the environmental envelop in these practice worlds?

@M1chaelM M1chaelM dismissed their stale review February 21, 2023 16:07

Removing this approval to avoid confusion until the environmental question is resolved.

@caguero
Copy link
Contributor Author

caguero commented Mar 15, 2023

@M1chaelM , the environmental conditions have been updated. This PR is ready for review.

Copy link
Collaborator

@M1chaelM M1chaelM left a comment

Choose a reason for hiding this comment

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

When I tested these 3 worlds it seemed like the wind was not having much of an effect on the WAMV, so my comments are directed at trying to understand how it works. On a related note, I think we used to have a coefficient vector specific to the WAMV--is this no longer necessary?

<force_approximation_scaling_factor>0.002</force_approximation_scaling_factor>
<horizontal>
<magnitude>
<time_for_rise>10</time_for_rise>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you help me understand these values? Our previous wind plugin had the following:

<wind_obj>
  <name>wamv</name>
  <link_name>wamv/base_link</link_name>
  <coeff_vector>.5 .5 .33</coeff_vector>
</wind_obj>
<wind_direction>240</wind_direction>
<wind_mean_velocity>0.0</wind_mean_velocity>
<var_wind_gain_constants>0</var_wind_gain_constants>
<var_wind_time_constants>2</var_wind_time_constants>
<random_seed>10</random_seed>
<update_rate>10</update_rate>

I think we are going to need to update our documentation about the environmental envelop to refer to the new values used here, but I don't yet understand the new ones. I looked at the plugin documentation but it is not much help.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the explanation of these parameters clear enough here?

https:/gazebosim/gz-sim/blob/gz-sim7/src/systems/wind_effects/WindEffects.hh#L35

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm sure it's clear enough for someone! I think I've almost got my head around it but I have a few questions:

  • What is the "user input"? Is that the linear velocity vector?
  • Am I right in thinking the "horizontal direction" is intended to add minor fluctuations in direction (to make it more realistic than the constant direction we used in VRX classic)?
  • It seems like the linear velocity vector is a combined representation of what used to be "mean velocity" and direction--is that right?
  • If so, am I right in thinking that the wind can now blow upward and downward, not just horizontally? It seems like a 3d vector allows us to specify some strange wind directions.
  • Also, it looks like the "vertical" component of the wind is treated differently from the horizontal component, in that there is no "vertical direction" effect. Do you know the reason for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Yes, the <wind><linear_velocity> SDF value or the WorldLinearVelocitySeed component.
  2. Exactly, that's there to add fluctuations in direction and/or magnitude.
  3. I think so.
  4. That's my intuition too.
  5. In the horizontal component makes sense to change direction because there are two components that effectively together compose the final direction. There's no direction in the vertical component because there's only one component here, the z.

<direction>
<time_for_rise>30</time_for_rise>
<sin>
<amplitude>5</amplitude>
Copy link
Collaborator

Choose a reason for hiding this comment

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

For example, is there a way to translate between these values and our previous wind_direction parameter given in degrees? It looks like this is a sin wave that causes the wind to fluctuate--is the initial direction is based on the linear_velocity vector?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My intuition is that this will modify the x and y components of the linea_velocitywind vector, i.e., its horizontal component.

</include>

<wind>
<linear_velocity>2000 0 0</linear_velocity>
Copy link
Collaborator

Choose a reason for hiding this comment

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

What are the units of this vector? I wonder if we should include a comment because the sdformat spec does not say.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd say meters per second but I haven't researched why the numbers are higher than before. I'd expect that the hydrodynamics plugin is adding some damping here but not sure if enough to make this number so high.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe the problem is the <force_approximation_scaling_factor>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I set the <force_approximation_scaling_factors> to 1 and now the wind linear velocities look more reasonable. Let me know what you think.

Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the force_approximation_scaling_factor intended to capture? Is this supposed to allow the wind to "feel stronger" in some regions than others? If so, setting it to 1 seems to make sense to me. (I will also test it--just trying to understand what it means.)

Signed-off-by: Carlos Agüero <[email protected]>
@M1chaelM
Copy link
Collaborator

@caguero I've been thinking about how to sum up my questions about the new wind effects. I think it will help to look at the documentation we provided in previous years, particularly section 7.3 of the Technical Guide, where we say:

"Wind exerts a force on objects in the VRX environment. The total wind velocity is a combination of a
constant mean velocity component and a variable wind speed (i.e., gusting). The variable component of the wind speed is modeled as a first-order linear spectrum defined by two components: the variability gain and the variability time constant. The variability gain specifies the magnitude (root-mean-square) of the variable component of the wind speed, and the variability time constant specifies how rapidly the wind speed changes with time. For details on the wind model and implementation see the Theory of Operations on the VRX Wiki For examples of how to change the wind parameters see the tutorial on Changing Simulation Parameters on the VRX Wiki."

Am I right in thinking that we have not only changed the way we specify wind parameters (so the above no longer applies) but have also changed the underlying model, so the section on wind in our paper also no longer applies? If so, do you know where we are getting our new model from? If not, I think we need to explain how the new parameter names relate to the model we described.

@caguero
Copy link
Contributor Author

caguero commented Mar 20, 2023

@caguero I've been thinking about how to sum up my questions about the new wind effects. I think it will help to look at the documentation we provided in previous years, particularly section 7.3 of the Technical Guide, where we say:

"Wind exerts a force on objects in the VRX environment. The total wind velocity is a combination of a constant mean velocity component and a variable wind speed (i.e., gusting). The variable component of the wind speed is modeled as a first-order linear spectrum defined by two components: the variability gain and the variability time constant. The variability gain specifies the magnitude (root-mean-square) of the variable component of the wind speed, and the variability time constant specifies how rapidly the wind speed changes with time. For details on the wind model and implementation see the Theory of Operations on the VRX Wiki For examples of how to change the wind parameters see the tutorial on Changing Simulation Parameters on the VRX Wiki."

Am I right in thinking that we have not only changed the way we specify wind parameters (so the above no longer applies) but have also changed the underlying model, so the section on wind in our paper also no longer applies? If so, do you know where we are getting our new model from? If not, I think we need to explain how the new parameter names relate to the model we described.

I think so too. It looks like the low pass filters are used to modify the horizontal and vertical components of the wind velocity vector.

@M1chaelM
Copy link
Collaborator

I think so too. It looks like the low pass filters are used to modify the horizontal and vertical components of the wind velocity vector.

OK, do you think you can track down the source of the theory behind this approach? It must be coming from a paper or a book, right? I'm tagging @bsb808 because I think this is a significant update to our environmental model.

@caguero
Copy link
Contributor Author

caguero commented Mar 21, 2023

I think so too. It looks like the low pass filters are used to modify the horizontal and vertical components of the wind velocity vector.

OK, do you think you can track down the source of the theory behind this approach? It must be coming from a paper or a book, right? I'm tagging @bsb808 because I think this is a significant update to our environmental model.

Perhaps we can move this discussion to the place where we update the competition documents?

@M1chaelM
Copy link
Collaborator

Perhaps we can move this discussion to the place where we update the competition documents?

Sure. The only hesitation I have is that I wanted to verify that our 3 scenarios give a reasonable coverage of the environmental envelope that the competitors should expect. Looking at last year's documentation, it seems like we have an envelop for the wave conditions but no limits at all relating to the wind, which is surprising, unless I'm looking in the wrong place.

For now, I think we could move on if we can provide a rough number representing roughly the maximum mean velocity of the wind that we expect teams to be able to operate in. @caguero @j-herman any suggestions?

@caguero
Copy link
Contributor Author

caguero commented Mar 22, 2023

For now, I think we could move on if we can provide a rough number representing roughly the maximum mean velocity of the wind that we expect teams to be able to operate in. @caguero @j-herman any suggestions?

It looks like a vector magnitude between 0 and 0.5 give a reasonable spectrum.

@M1chaelM
Copy link
Collaborator

It looks like a vector magnitude between 0 and 0.5 give a reasonable spectrum.

I definitely agree with the lower bound. Can you say where you're getting the 0.5? If that is meters per second, it's about 1 knot, right? Or am I making a mistake on the conversion. It seems kind of low, and the subjective experience of testing also made me feel like the wind isn't having as much of an effect as I'd expect (even after correcting the scaling factor).

@j-herman
Copy link
Collaborator

@M1chaelM I spent some time reviewing the previous model and looking at the new plugin. They are significantly different in both the spectrum generation and in the method of application to the WAMV. Changing the spectrum seems fine, although it would be nice to know if the new plugin is based on some existing model of wind variability or is generic. I do not see a straightforward way to map the test data we have for the WAMV (reference 25 in the paper is the source) to the new plugin's force application method. So, we may have to decide between reverting to a specific surface-vessel wind plugin that uses the 3-DOF coefficient-based method, or using the new plugin and experimenting with parameters to find some combinations that yield reasonable behavior. This may be fine for VRX competition purposes but would be good to document for other users of the simulation.
As far as the units discussion, I don't think there are any true units associated with the wind plugin. It's all accounted for in the scale factor. We'll have to decide what we want the units to be and then scale the WAMV's response to make it "right" for those units.

@M1chaelM
Copy link
Collaborator

I do not see a straightforward way to map the test data we have for the WAMV (reference 25 in the paper is the source) to the new plugin's force application method. So, we may have to decide between reverting to a specific surface-vessel wind plugin that uses the 3-DOF coefficient-based method, or using the new plugin and experimenting with parameters to find some combinations that yield reasonable behavior.

@caguero In light of @j-herman 's comment above I'm feeling hesitant to move to this new wind model because our old one was based on at least some empirical data specific to the wamv. This raises at least two questions:

  1. Is there (or will there be) a way to incorporate WAMV-specific coefficients into the new generic wind plugin?
  2. What level of effort would be required to preserve the previous model?

In any case, I think it would also help to know what the new model is based on.

@caguero
Copy link
Contributor Author

caguero commented Mar 22, 2023

It looks like a vector magnitude between 0 and 0.5 give a reasonable spectrum.

I definitely agree with the lower bound. Can you say where you're getting the 0.5? If that is meters per second, it's about 1 knot, right? Or am I making a mistake on the conversion. It seems kind of low, and the subjective experience of testing also made me feel like the wind isn't having as much of an effect as I'd expect (even after correcting the scaling factor).

Empirically, running the simulation with different values. We can definitely increase that number if we feel is too low.

@M1chaelM
Copy link
Collaborator

I don't think there are any true units associated with the wind plugin. It's all accounted for in the scale factor. We'll have to decide what we want the units to be and then scale the WAMV's response to make it "right" for those units.

@j-herman I'm still getting caught up on this, but a unitless linear velocity vector seems a bit strange, whereas I would usually assume that a "scaling factor" of 1 is supposed to indicate no scaling. I will keep digging--I'm sure there was some intention of making these parameters relate to environmental conditions in the real world. I think the documentation is just still a work in progress.

@j-herman
Copy link
Collaborator

j-herman commented Mar 22, 2023

I don't think there are any true units associated with the wind plugin. It's all accounted for in the scale factor. We'll have to decide what we want the units to be and then scale the WAMV's response to make it "right" for those units.

@j-herman I'm still getting caught up on this, but a unitless linear velocity vector seems a bit strange, whereas I would usually assume that a "scaling factor" of 1 is supposed to indicate no scaling. I will keep digging--I'm sure there was some intention of making these parameters relate to environmental conditions in the real world. I think the documentation is just still a work in progress.

It is a bit strange, but the scaling factor appears to be something like a wind resistance level assigned to an individual link - and, without knowing what a factor of 1 is meant to represent at a physical level, we can choose to set one parameter or the other as we like. If we dig in and find out that "1" represents the force experienced by, say, a 1 kg sphere of 1 meter in diameter that experiences a 1 kt wind, then we will have some way to define units. Of course, I may be misreading the plugin entirely.
I'll do a more in-depth read of the code and see where the conversion to force/ torque happens, and hopefully that will give more insight into the inputs.

Update: We need the magnitude to be in the same units as the velocity of individual links. I should have realized this before but I hadn't thought through the problem where the vehicle is moving rather than stationkeeping. So, we will have to use only the scaling factor to adjust the force experienced by the links to an appropriate amount.

@j-herman
Copy link
Collaborator

Just to be clear on how I'm thinking about this - I don't know whether one method produces a more realistic wind effect than the other, and I'm not recommending that we revert back at this point. If we can find a reference for the new plugin's methodology we can use that as the basis for a new technical description - one that might be physics-based rather than coming from test data. We could then validate with spot checks against the original wind model.

@caguero
Copy link
Contributor Author

caguero commented Apr 5, 2023

As per our offline conversation, I have updated the wind parameters in all the outstanding PRs:

  • The <force_approximation_scaling_factor> has been set to 1.
  • The minimum wind linear velocity is no 0 m/s (no wind).
  • The maximum wind linear velocity is 2 m/s.

I think these values are now not too far from the values that we had in VRX classic. In VRX Classic, the max wind was around 7 m/s with a vector coefficient vector of {.5 .5 .33}.

@M1chaelM
Copy link
Collaborator

M1chaelM commented Apr 7, 2023

OK, so I think our settings right now are:

World 0

No wind.

World 1

  • Mean: 1 m/s
  • Direction: parallel to x axis (0 degrees)

World 2

  • Mean: 2 m/s
  • Direction: parallel to x axis (0 degrees)

Effects (all worlds):

Horizontal Magnitude

  • Time for rise: 10
  • Sin Amplitude percent: 0.05
    • Is this supposed to indicate 5%?
  • Sin Period: 60

Horizontal Direction

  • Time for rise: 30
  • Sin Amplitude: 5
  • Sin Period: 20

Vertical Magnitude

  • No effects (0 mean)

It is still a little difficult for me to understand the effects settings because the documentation describes everything in terms of analogies to low pass filters. However, from looking at the source code it seems that the directional "amplitude" is actually specified in degrees. So, roughly, I think our wind effect should be causing the magnitude of the wind to be varied from %95 to %105 of the value calculated from the linear velocity vector, and the direction should vary +/- 5 degrees.

Do you agree?

EDIT: After reviewing this again I'm convinced we will need to port our custom wind plugin from "vrx classic," so these values will become moot anyway. It is helpful that they are roughly similar to what the final effects will be for these practice worlds, but I think we (I) can forgo further inquiry into the meaning of the parameters.

Copy link
Collaborator

@M1chaelM M1chaelM left a comment

Choose a reason for hiding this comment

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

Approving under the assumption that we will port our prior wind plugin before the final release.

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.

3 participants