Skip to content

[FEAT] Implemented covariance calculation Closes #48 #66

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

jiyo4476
Copy link
Contributor

  • Covariance Test cases
  • NDArray::cov function in numpower to expose to PHP
  • Covariance implementation without Hstack since not in branch

Submission Checklist:

Due to the inherent complexity of this library, we created this checklist to remind everyone of the essential steps to have an MR approved depending on the type of change that is made. You can delete this.

  • [ X ] Have you followed the guidelines in our Contributing document?
  • [ X ] Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • [ X ] Does your submission pass tests with ZEND_ALLOC enabled? export USE_ZEND_ALLOC=1 && make test
  • [ X ] Does your submission pass tests with ZEND_ALLOC disabled? export USE_ZEND_ALLOC=0 && make test

Change to methods and operations

  • [ X ] Have you verified that your change does not break backwards compatibility?
  • [ X ] Have you updated the operation(s) tests for your changes, as applicable?
  • Optional: Have your changes also been tested on the GPU? If you don't have a GPU available, you'll need to wait for a community member to perform the approval with a GPU. This only applies to changes that can affect GPU functionality.
  • Optional: Have your changes also been tested on the GPU with the NDARRAY_VCHECK option enabled and no VRAM memory leaks were displayed? export NDARRAY_VCHECK=1 && make test

+ Covariance Test cases
+ NDArray::cov function in numpower to expose to PHP
+ Covariance implementation without Hstack since not in branch
@jiyo4476 jiyo4476 changed the title Implemented covariance calculation [FEAT] Implemented covariance calculation [Issues # 48] Sep 26, 2024
@jiyo4476 jiyo4476 changed the title [FEAT] Implemented covariance calculation [Issues # 48] [FEAT] Implemented covariance calculation Closes #48 Sep 26, 2024
Combined transpose and matrix multiplication in Covariance function.
Fixed naming of NDArray param for covariance function
- Cleaned up unused variables for covariance exposure function in numpower.c
+ added argument for inputting matrix with the rows defined as cols
+ Refactored the NDArray_cov function in statistics.c to utilize the slice method for data extraction.
+ Replaced manual data copying with the slice method to improve code readability and maintainability.
+ Verified that the function maintains its original functionality and correctness after the refactor.
@henrique-borba henrique-borba self-requested a review September 30, 2024 20:07
@henrique-borba henrique-borba self-assigned this Sep 30, 2024
@henrique-borba henrique-borba added the enhancement New feature or request label Sep 30, 2024
- Removed the unused a_data array
- Removed the unused col_shape array
- Ensured that the function maintains its original functionality and correctness.
- Verified that all tests pass successfully, confirming the correctness of the changes.
+ Changed name of norm_a to centered_a
+ Changed name of norm_vectors to centered_vectors
- Ensured that the function maintains its original functionality and correctness.
- Verified that all tests pass successfully, confirming the correctness of the changes.
+ Replaced efree calls with NDArray_FREE for NDArrays in the covariance function to ensure proper memory management.
- Ensured that the function maintains its original functionality and correctness.
- Verified that all tests pass successfully, confirming the correctness of the changes.
@jiyo4476 jiyo4476 force-pushed the FEAT-48-Implement-covariance-calculation branch from f696e4c to af8c992 Compare October 3, 2024 02:32
…ered variable

+ Refactored the covariance function in statistics.c to combine the mean calculation and subtraction into a single centered variable.
+ Simplified code by removing the intermediate mean and subtracted variables
- Ensured that the function maintains its original functionality and correctness.
- Verified that all tests pass successfully, confirming the correctness of the changes.
… time

+ Refactored the covariance function in statistics.c to move the allocation of the centered_vectors pointer array closer to its usage.
+ Reduced the time centered_vectors exists in the heap, improving memory management
- Ensured that the function maintains its original functionality and correctness.
- Verified that all tests pass successfully, confirming the correctness of the changes.
@SkibidiProduction
Copy link
Contributor

so, will this PR be merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants