-
Notifications
You must be signed in to change notification settings - Fork 0
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 some more (slightly of the codebase changed) speedups #1
Conversation
to_concatenate = np.array([convert_type(padding_value()) if callable(padding_value) else convert_type(padding_value) for _ in range(len(lst), length)]) | ||
padding = convert_type(padding_value()) if callable(padding_value) else convert_type(padding_value) | ||
to_concatenate = np.array([padding for _ in range(len(lst), length)]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This defeats the purpose of having padding_value be a callable. We don't want to reuse the same instance of the padding value, that's why we we pass in a callable as the argument. Unless something has changed since I wrote the code originally, reusing the same instance of that padding value might break some stuff.
l = np.array([convert_type(padding_value()) if callable(padding_value) else convert_type(padding_value) for _ in range(length)]) | ||
padding = convert_type(padding_value()) if callable(padding_value) else convert_type(padding_value) | ||
l = np.array([padding for _ in range(length)]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing here
type_mapping: list[tuple[typing.Type, typing.Callable]] = [ | ||
(bool, convert_bool), | ||
(dict, convert_dict), | ||
(list, convert_list), | ||
(str, convert_str), | ||
(PositionData, convert_positiondata), | ||
(TaskData, convert_taskdata), | ||
(Vector2, convert_vector2), | ||
(betterproto.Message, convert_message), | ||
(betterproto.Enum, convert_enum), | ||
(int, lambda x: [x]), | ||
(float, lambda x: [x]), | ||
(np.ndarray, lambda x: convert_list(x.tolist())), | ||
] | ||
|
||
#print(extra_info) | ||
|
||
for cls, func in type_mapping.items(): | ||
for cls, func in type_mapping: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is faster though you can include it in the other pr
ty @Alexejhero, was unsure if these would have a negative effect in places given I didn't have much experience. The other speedup in this pr has very little benefit compared to this part. |
(I'm using another device so these numbers aren't relative to the timings)
Current speedups
With a list of tuples instead of a dict
Re-using values in
pad_list