-
Notifications
You must be signed in to change notification settings - Fork 8
[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
jiyo4476
wants to merge
10
commits into
NumPower:main
Choose a base branch
from
jiyo4476:FEAT-48-Implement-covariance-calculation
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
[FEAT] Implemented covariance calculation Closes #48 #66
jiyo4476
wants to merge
10
commits into
NumPower:main
from
jiyo4476:FEAT-48-Implement-covariance-calculation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
+ Covariance Test cases + NDArray::cov function in numpower to expose to PHP + Covariance implementation without Hstack since not in branch
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.
- 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.
f696e4c
to
af8c992
Compare
…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.
so, will this PR be merged? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
export USE_ZEND_ALLOC=1 && make test
export USE_ZEND_ALLOC=0 && make test
Change to methods and operations
NDARRAY_VCHECK
option enabled and no VRAM memory leaks were displayed?export NDARRAY_VCHECK=1 && make test