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

Add getters/setters for radii and diameters of EllipsoidShape #829

Merged
merged 4 commits into from
Jan 14, 2017

Conversation

jslee02
Copy link
Member

@jslee02 jslee02 commented Dec 22, 2016

DART currently uses diameters for the primary properties of EllipsoidShape, which is not the commonly used convention. This PR is preliminary changes for replacing diameters with radii that include:

  • deprecate get/setSize
  • add get/setDiameter
  • add get/setRadii

In the next major release, diameters will be totally replaced by radii.


This change is Reviewable

@jslee02 jslee02 added this to the DART 6.2.0 milestone Dec 22, 2016
@psigen
Copy link
Collaborator

psigen commented Dec 22, 2016

Wait, I have some confusion here.

I thought the conclusion in #827 was that semi-principal axis lengths (diameters) was the convention, but that the word radii was used to describe these lengths in certain places.

(I am OK with either, since the major change is simply making the notation consistent, I just want to verify that this is the intent.)

@jslee02
Copy link
Member Author

jslee02 commented Jan 11, 2017

Well, maybe I'm the one has confusion here. I understand that the lengths along the semi-principal axes (i.e., a, b, and c) represent the radii and that's the convention. Please correct me if I'm wrong.

@psigen
Copy link
Collaborator

psigen commented Jan 12, 2017

Ok, I think the confusion stems from the following:

  1. The lengths of the semi-principal axes are typically represented in diameters (distance of a line segment from one side to the other, passing through the center of the ellipse.
  2. The notation in DART describes ellipses in terms of radii, which usually means 1/2 of the diameter.

@ClintLiddick
Copy link

@psigen: @jslee02 and I talked, looked through wikipedia and the linked issue, and it seems like there was a miscommunication. Below is what we believe is correct.

  1. Ellipsoids are most commonly parameterized by their semi-principal axes, which is equivalent to (but not called) the radii (i.e. they go from the center, through a focus, to an edge)
  2. Principal axes are equivalent to diameter (from edge to edge, through center and foci)
  3. Dart currently uses principle axes, but calls them diameter.
  4. Dart wants to transition to using semi-principal axes, and in this PR (technically incorrectly) refers to it as this radii.

Dart could use either the principal or semi-principal axes, but should term them as such to avoid confusion.

@psigen
Copy link
Collaborator

psigen commented Jan 12, 2017

Ah, this makes sense. I think both JS and I had confusion around various terminology.

@jslee02 jslee02 merged commit 8307ed0 into master Jan 14, 2017
@jslee02 jslee02 deleted the ellipsoid_radii branch January 14, 2017 11:44
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