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

min_size_per_dim + changes for scalars inputs+outputs support #354

Merged
merged 109 commits into from
Jul 26, 2024
Merged

Conversation

YoelShoshan
Copy link
Collaborator

@YoelShoshan YoelShoshan commented May 22, 2024

by allowing min_size_per_dim to be longer than the number of dims in a tensor, it makes usage easier.

for example,

min_size_per_dim=(10_000, -1)
would then work for tensors of single dim - e.g. of size (1234, )
and also for tensors of 2(+) dims - e.g. of size (4321, 100)

this is much more nice for the user in several cases.

also - few changes for scalars inputs+outputs

Yoel Shoshan and others added 30 commits April 14, 2022 17:13
… keys() items() and values() to our NDict, and tried to highlight more the faulting op in pipeline ops error
* Changed default deepdiff behavior to ignore nans in comparison,
* Added keys() items() and values() to our NDict (until now it returned empty iterables for those which is incorrect)
* Tried to highlight more the faulting op in pipeline ops error
…le_ids. fixed a typo in samplers.py and added a describe method to NDict
@YoelShoshan YoelShoshan requested a review from mosheraboh May 22, 2024 08:06
mosheraboh
mosheraboh previously approved these changes May 22, 2024
Copy link
Collaborator

@mosheraboh mosheraboh left a comment

Choose a reason for hiding this comment

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

LGTM

@YoelShoshan YoelShoshan changed the title more flexible min_size_per_dim changes for scalars inputs+outputs support Jul 24, 2024
@YoelShoshan YoelShoshan changed the title changes for scalars inputs+outputs support min_size_per_dim + changes for scalars inputs+outputs support Jul 24, 2024
@YoelShoshan YoelShoshan requested a review from mosheraboh July 24, 2024 08:05
mosheraboh
mosheraboh previously approved these changes Jul 25, 2024
Copy link
Collaborator

@mosheraboh mosheraboh left a comment

Choose a reason for hiding this comment

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

Looks good.
Minor comments inline.

@@ -37,12 +37,15 @@ def to_tensor(
value: Any,
dtype: Optional[torch.dtype] = None,
device: Optional[torch.device] = None,
ignore_None: bool = False,
Copy link
Collaborator

Choose a reason for hiding this comment

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

ignore_none

if np.isscalar(pred[0]):
pred = np.array(pred)
else:
pred = np.concatenate(pred)
Copy link
Collaborator

Choose a reason for hiding this comment

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

here pred is numpy? necessarily same dims?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, in the else it's numpy, and it not necessarily same dims (it's 1d each)

…ccumulation, and also to make pearson correlation calculation to support the case of empty inputs
@YoelShoshan YoelShoshan requested a review from mosheraboh July 26, 2024 09:43
@YoelShoshan YoelShoshan merged commit 27a89a7 into master Jul 26, 2024
5 checks passed
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