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

Octahedron in dipy.core.triangle_subdivide has wrong faces #15

Closed
Garyfallidis opened this issue Nov 5, 2011 · 7 comments
Closed

Octahedron in dipy.core.triangle_subdivide has wrong faces #15

Garyfallidis opened this issue Nov 5, 2011 · 7 comments

Comments

@Garyfallidis
Copy link
Contributor

This are the correct vertices and faces of the octahedron

octahedron_vertices = np.array( [
[ 1.0, 0.0, 0.0], # 0
[-1.0, 0.0, 0.0], # 1
[ 0.0, 1.0, 0.0], # 2
[ 0.0,-1.0, 0.0], # 3
[ 0.0, 0.0, 1.0], # 4
[ 0.0, 0.0,-1.0] # 5
],dtype=np.float32)

octahedron_triangles = np.array( [
[ 0, 2, 4],
[ 2, 5, 0],
[ 1, 4, 2],
[ 1, 2, 5],
[ 3, 4, 0],
[ 3, 0, 5],
[ 3, 1, 4],
[ 1, 3, 5]
], dtype=np.uint32)

Best wishes,
Eleftherios

@MrBago
Copy link
Contributor

MrBago commented Nov 6, 2011

Sorry, I should make this more clear in the documentation.

octahedron_triangles is the edges of the faces of the octahedron not the vertices of the octahedron, to get the vertices you must use octahedron_edges[octahedron_triangles, 0](octahedron_edges[octahedron_triangles, 1] will also work and the fact that both of these work is quite special and very important to how the function divide_all works).

I realize this is confusing, but by doing it this way, we are able to use the divide_all algorithm to create a unique set of vertecies on the unit sphere. One of the reasons we might want our set of verticies to be unique in so we can run electrostatic repulsion on them.

Bago

@MrBago MrBago closed this as completed Nov 6, 2011
@iannimmosmith
Copy link
Contributor

Who will this have affected?

Ian

On 5 November 2011 21:35, Eleftherios Garyfallidis
reply@reply.github.com
wrote:

This are the correct vertices and faces of the octahedron

octahedron_vertices = np.array( [
   [ 1.0, 0.0, 0.0], # 0
   [-1.0, 0.0, 0.0], # 1
   [ 0.0, 1.0, 0.0], # 2
   [ 0.0,-1.0, 0.0], # 3
   [ 0.0, 0.0, 1.0], # 4
   [ 0.0, 0.0,-1.0]  # 5
   ],dtype=np.float32)

octahedron_triangles = np.array( [
   [ 0,  2,  4],
   [ 2,  5,  0],
   [ 1,  4,  2],
   [ 1,  2,  5],
   [ 3,  4,  0],
   [ 3,  0,  5],
   [ 3,  1,  4],
   [ 1,  3,  5]
   ], dtype=np.uint32)

Best wishes,
Eleftherios


Reply to this email directly or view it on GitHub:
#15

@Garyfallidis
Copy link
Contributor Author

Ian you can see who changed things in a file using the command

git blame

It seems that the name was misleading that is all :-)

Bago I saw that you closed the issue but would it be okay to change that name into something that is more explanatory or at least add a comment?

@iannimmosmith
Copy link
Contributor

Neat!

Ian

On 6 November 2011 11:05, Eleftherios Garyfallidis
reply@reply.github.com
wrote:

Ian you can see who changed things in a file using the command

git blame

It seems that the name was misleading that is all :-)

Bago I saw that you closed the issue but would it be okay to change that name into something that is more explanatory or at least add a comment?


Reply to this email directly or view it on GitHub:
#15 (comment)

@MrBago
Copy link
Contributor

MrBago commented Nov 6, 2011

Just FYI, this is from the documentation for create_unit_sphere, but I can also add comment where the octahedron is defined to avoid further confusion.

Returns
---------
vertices : array
    A 2d array with the x, y, and z coordinates of vertices on a unit
    sphere.
edges : array
    A 2d array of vertex pairs for every set of neighboring vertexes
    on a unit sphere.
triangles : array
    A 2d array of edge triplets representing triangles on the surface of a
    unit sphere.

On a related note, are you guys using the icosahedron that's currently in this module? If I remember correctly from when I looked at this, I believe it is not possible to use an icosahedron with the current divide_all function, has someone found a way to do so?

Bago

@Garyfallidis
Copy link
Contributor Author

Cool, yes please comment also the octahedron, perhaps you should also check the icosahedron. If someone shouldn't use it then it shouldn't be there. GGT!!! :-)

@iannimmosmith
Copy link
Contributor

... and pushed

Garyfallidis added a commit that referenced this issue Mar 24, 2013
Added the last pull request to the gh change-log and fixed a typo.
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

No branches or pull requests

3 participants