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

using rowvar keyword in dpnp.cov #1371

Merged
merged 1 commit into from
Apr 15, 2023
Merged

using rowvar keyword in dpnp.cov #1371

merged 1 commit into from
Apr 15, 2023

Conversation

vtavana
Copy link
Collaborator

@vtavana vtavana commented Apr 5, 2023

Closes #1367

rowvar keyword is added to dpnp.cov function

dpnp.dpnp_array.dpnp_array._create_from_usm_ndarray(x.get_array().mT) is used since it is faster than dpnp.transpose(x) since dpnp.tarnsport
%timeit dpnp.cov(dpnp.dpnp_array.dpnp_array._create_from_usm_ndarray(x.get_array().mT))
5.4 ms ± 121 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

%timeit dpnp.cov(dpnp.transpose(x))
9.46 ms ± 455 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

%timeit dpnp.cov(x, rowvar=False)
5.49 ms ± 352 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • Have you checked performance impact of proposed changes?
  • If this PR is a work in progress, are you filing the PR as a draft?

@vtavana vtavana force-pushed the add_rowvar_to_cov branch from eb6c4b5 to 11d596f Compare April 5, 2023 19:21
@vtavana vtavana requested a review from antonwolfy April 5, 2023 19:26
@vtavana vtavana force-pushed the add_rowvar_to_cov branch 2 times, most recently from 16240a1 to bdd40da Compare April 6, 2023 14:43
@vtavana vtavana changed the title using rowvar flag in dpnp.cov using rowvar keyword in dpnp.cov Apr 6, 2023
@vtavana vtavana force-pushed the add_rowvar_to_cov branch from bdd40da to 7b3b992 Compare April 6, 2023 19:35
dpnp/dpnp_iface_statistics.py Outdated Show resolved Hide resolved
dpnp/dpnp_iface_statistics.py Outdated Show resolved Hide resolved
@vtavana vtavana force-pushed the add_rowvar_to_cov branch from 7b3b992 to 5d837b1 Compare April 10, 2023 13:44
tests/test_statistics.py Outdated Show resolved Hide resolved
dpnp/dpnp_iface_statistics.py Outdated Show resolved Hide resolved
tests/test_statistics.py Outdated Show resolved Hide resolved
@vtavana vtavana force-pushed the add_rowvar_to_cov branch from 5d837b1 to 4e04716 Compare April 12, 2023 19:31
if not rowvar and x1.shape[0] != 1:
x1 = x1.get_array() if isinstance(x1, dpnp_array) else x1
x1 = dpnp_array._create_from_usm_ndarray(x1.mT)
x1 = dpnp.astype(x1, dpnp.float64) if x1_desc.dtype != dpnp.float64 else x1
Copy link
Contributor

Choose a reason for hiding this comment

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

Please test that this works on Iris Xe, or other devices that do not support float64 data type.

Copy link
Collaborator Author

@vtavana vtavana Apr 13, 2023

Choose a reason for hiding this comment

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

It will not work. @antonwolfy mentioned it is written like this because Numpy by default returns float64 for any input data type. The suggestion was to fix it in a separate ticket since it needs to correct dpnp c++ backend code.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've filed #1379 to add support of dpnp.cov on Iris Xe

x1 = dpnp.astype(x1, dpnp.float64) if x1_desc.dtype != dpnp.float64 else x1
x1_desc = dpnp.get_dpnp_descriptor(x1, copy_when_nondefault_queue=False)
elif x1_desc.dtype != dpnp.float64:
x1 = dpnp.astype(x1, dpnp.float64)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment here

@vtavana vtavana force-pushed the add_rowvar_to_cov branch from 4e04716 to 93bb5e7 Compare April 14, 2023 14:17
@vtavana vtavana force-pushed the add_rowvar_to_cov branch from 93bb5e7 to d7c3dc8 Compare April 14, 2023 16:35
Copy link
Contributor

@antonwolfy antonwolfy left a comment

Choose a reason for hiding this comment

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

LGTM! thank you @vtavana !

@antonwolfy antonwolfy merged commit 2dfa804 into master Apr 15, 2023
@antonwolfy antonwolfy deleted the add_rowvar_to_cov branch April 15, 2023 11:23
@vtavana vtavana self-assigned this Apr 25, 2023
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.

dpnp.cov should support rowvar keyword argument
3 participants