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

Confusing names for torus parameters #3177

Closed
eshmoylova opened this issue Oct 28, 2019 · 4 comments
Closed

Confusing names for torus parameters #3177

eshmoylova opened this issue Oct 28, 2019 · 4 comments
Assignees
Labels
L: Mechanics.MultiBody Issue addresses Modelica.Mechanics.MultiBody
Milestone

Comments

@eshmoylova
Copy link
Member

eshmoylova commented Oct 28, 2019

Modelica.Mechanics.MultiBody.Visualizers.Advanced.SurfaceCharacteristics.torus ans Modelica.Mechanics.MultiBody.Visualizers.Torus use confusing names for the description of the parameters: ri and ro.
Visualizers.Torus defines:

parameter Modelica.SIunits.Radius ri=0.5 "Inner radius of torus" annotation(Dialog(enable=animation));
parameter Modelica.SIunits.Radius ro=0.1 "Outer radius of torus (=width/2)"

And SurfaceCharacteristics.torus defines:

input Modelica.SIunits.Radius ri=1 "Inner radius of torus" annotation(Dialog);
input Modelica.SIunits.Radius ro=0.2 "Outer radius of torus (=width/2)" annotation(Dialog);

Usually, the inner radius for a torus is usually defined as the radius of the hole and the outer one as the radius of the whole shape. For example, see this. So having inner radius to be greater than the outer one does not make sense.

Looking at the formula used in SurfaceCharacteristics, it looks like ri is used as the major radius and ro as the minor radius, as defined here.

It would be good if there was perhaps some reference given to how ri and ro are supposed to be interpreted.

@eshmoylova eshmoylova added the L: Mechanics.MultiBody Issue addresses Modelica.Mechanics.MultiBody label Oct 28, 2019
@MartinOtter
Copy link
Member

Simplest probably to change the description texts.

  • ri: Distance from the center of the tube to the center of the torus,
  • ro: Radius of the tube.

Would be good to add a figure to make this clear.

I agree, the naming of the parameters could be better. However, names such as major/minor radius are also not completely obvious.

@tobolar
Copy link
Contributor

tobolar commented Nov 22, 2019

  • ri: Distance from the center of the tube to the center of the torus,
  • ro: Radius of the tube.

The description is fine. We can even use:

  • ri: Major radius: distance from the center of the tube to the center of the torus,
  • ro: Minor radius: radius of the tube.

Unfortunately, the parameter names riand roare confusing then since they imply thinking of inner and outer radii which is definitely false. Following Wikipedia, we could use

  • R: Major radius: distance from the center of the tube to the center of the torus,
  • r: Minor radius: radius of the tube.

Unfortunately, WolframMathWorld uses exactly those names for indicating the inner and the outer radius. :-(
Thus

Would be good to add a figure to make this clear.

is a must in this case.

@beutlich
Copy link
Member

I am in favour to follow Wikipedia and go for:

  • R: Major radius (distance from the center of the tube to the center of the torus)
  • r: Minor radius (radius of the tube)

@beutlich beutlich added this to the MSL4.0.0 milestone Dec 4, 2019
@beutlich
Copy link
Member

beutlich commented Dec 4, 2019

I believe that this also was resolved by #3176.

@beutlich beutlich closed this as completed Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: Mechanics.MultiBody Issue addresses Modelica.Mechanics.MultiBody
Projects
None yet
Development

No branches or pull requests

4 participants