Skip to content

Includes code for doing batched transforms in both directions#1025

Open
apasarkar wants to merge 5 commits intondwidgetfrom
update_coord_transforms
Open

Includes code for doing batched transforms in both directions#1025
apasarkar wants to merge 5 commits intondwidgetfrom
update_coord_transforms

Conversation

@apasarkar
Copy link
Copy Markdown
Collaborator

This PR allows you to do batched coordinate transforms. You have a graphic, the graphic has data points stored in model space. You want to transform all of those model points to world space (or go from world space to model space). Easy to batch/vectorize this via pylinalg but the fpl functions didn't allow for this.

You can do this now:

my_data = graphic.data[:] # Let's say this is shape (100, 3)
world_coords = graphic.map_model_to_world(my_data) 

graphic.map_world_to_model also supports this now

Comment on lines +532 to +534
if len(position) == 2:
# use z of the graphic
position = [*position, self.offset[-1]]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if this is only used for model <-> world, we should enforce that z should always be provided.

if they want to do model -> world then using the world z (which is the offset) will give the wrong result

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yep was thinking this

Comment on lines +542 to +544
raise ValueError(
f"position can either be shape (num_points, [2, 3]) or ([2, 3],)"
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"position must be of shape [3] or [n, 3]"

Comment on lines +548 to +553
if position.shape[-1] == 2:
z_data = np.zeros((position.shape[0], 1))
z_data[:] = self.offset[-1]
position = np.concatenate(
[position, z_data], axis=1
) # Shape (num_points, 3)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we shouldn't guess a z, user should provide it always

Converts position data (in the form of tuple or np.ndarray) into a (num_points, 3)-shaped np.ndarray for processing
"""

if isinstance(position, tuple):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just convert it to array at the beginning, np.asarray(position) and use one code block.

) # Shape (num_points, 3)
elif position.shape[-1] != 3:
raise ValueError(
"position must be a tuple or array indicating (x, y, z). The last dimension should be either 2 or 3"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[x, y, z] position or [n, 3]

Converts position data (in the form of tuple or np.ndarray) into a (num_points, 3)-shaped np.ndarray for processing
"""

if isinstance(position, tuple):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No if clause, always call np.asarray, it's free if the obj is already an array.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

"""
Converts position data (in the form of tuple or np.ndarray) into a (num_points, 3)-shaped np.ndarray for processing
"""
if not isinstance(position, (np.ndarray, tuple)):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just use np.asarray with no type checks, if numpy can't convert it to an array it'll raise, so it'll also handle a list etc.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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.

2 participants