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.

Sign up for GitHub

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

Jump to bottom

[FEAT] Implement NDArray::cumprod and NDArray::cumsum Closes #45 & Closes #46 #67

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

[FEAT] Implement NDArray::cumprod and NDArray::cumsum Closes #45 & Closes #46 #67

wants to merge 10 commits into from

Conversation

jiyo4476
Copy link
Contributor

@jiyo4476 jiyo4476 commented Sep 28, 2024 •
edited
Loading

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

Changes to Core Components:

This include changes to: buffer.c, gpu_alloc.c, ndarray.c, iterators.c and their associated header files.

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Your change does not affect the GPU, or if it does, it was tested with the GPU and the NDARRAY_VCHECK environment variable set and with no VRAM memory leaks warnings?

@jiyo4476
Add test cases for NDArray cumprod function
b4c93a9 
This commit adds comprehensive test cases to ensure the correctness of the cumprod function. The new tests cover a variety of input scenarios, including:

- Empty arrays
- Arrays with a single element
- Arrays with multiple elements
- Inputs with different axis
@jiyo4476
**Commit Subject:** Fix test cases for default implementation and axi…
a8ae369 
…s 0 in ndarray cumprod **Commit Description:** - Updated test cases to correctly validate the cumulative product function for the default implementation. - Added test cases to verify the cumulative product along axis 0. - Ensured that the expected output matches the actual output for both scenarios. - Refactored test data to improve readability and maintainability.

**Commit Subject:**
Fix test cases for default implementation and axis 0 in ndarray cumprod

**Commit Description:**
- Updated test cases to correctly validate the cumulative product function for the default implementation.
- Added test cases to verify the cumulative product along axis 0.
- Ensured that the expected output matches the actual output for both scenarios.
- Refactored test data to improve readability and maintainability.
@jiyo4476
Add declaration and base empty definition for NDArray_Cum_Prod_Float …
83c0da9 
…**Commit Description:** - Added the function declaration for `NDArray_Cum_Prod_Float` in [`arithmetics.h`](command:_github.copilot.openRelativePath?%5B%7B%22scheme%22%3A%22file%22%2C%22authority%22%3A%22%22%2C%22path%22%3A%22%2FC%3A%2FUsers%2Fjimmy%2Fgithub%2Fnumpower%2Fnumpower%2Fsrc%2Fndmath%2Farithmetics.h%22%2C%22query%22%3A%22%22%2C%22fragment%22%3A%22%22%7D%5D "c:\Users\jimmy\github\numpower\numpower\src\ndmath\arithmetics.h"). - Created a base empty definition for `NDArray_Cum_Prod_Float` to serve as a placeholder for future implementation. - Ensured the code compiles without errors after adding the new function declaration and definition

+ Added the function declaration for NDArray_Cum_Prod_Float in arithmetics.h
+ Created a base empty definition for NDArray_Cum_Prod_Float to serve as a placeholder for future implementation.
@jiyo4476
Register cumProd as PHP function and update declaration/definition to…
b576717 
… support all data types

- Registered the cumProd method as a PHP function to enable its usage in PHP scripts
- Modified the declartion and definition of NDArray_Cum_Prod to support various data types and to take pointer in for axis
@jiyo4476
Implemented cumProd function and update tests for correct output
27a1915 
+ Implemented the cumProd function in arithmetics.c to compute the cumulative product along the specified axis.
+ Added logic to handle cumulative product computation for both flattened arrays and along specified axes (0 and 1).
+ Updated test suite to verify the correct output of the cumProd function.
- Verified that all tests pass successfully, confirming the correctness of the implementation.
@jiyo4476
Vectorize cumProd function for improved performance
6cdb0ae 
+ Refactored the cumProd function in arithmetics.c to use vectorized operations for improved performance.
+ Verified that the refactored function produces the correct cumulative product results
@jiyo4476
Refactored the cumprod function into multiple functions for later use…
6cb6f62 
… with cumSum

+ Refactored the NDArray_Cum_Prod to multiple functions allowing for operations to be passed as parameters.
+ Introduced NDArray_Cum_Axis and NDArray_Cum_Flat to handle cumulative operations.
+ Added float_sum and float_product to allow them to be passed as parameters for NDArray_Cum_Flat and NDArray_Cum_Axis.
+ Ensured that the refactored functions maintain their original functionality and correctness.
+ Verified that all tests pass successfully, confirming the correctness of the refactor and new functionality.
@jiyo4476
Add cumulative sum function and expose it to PHP; include test cases
afbbab3 
+ Added the NDArray_Cum_Sum function to perform cumulative sum operations on NDArray objects.
+ Exposed the NDArray_Cum_Sum function to PHP via the cumSum method in numpower class.
+ Implemented the cumSum method to handle optional axis parameters, where the default operation will be on a flattened matrix.
+ Ensured that the new functionality integrates seamlessly with existing NDArray operations and maintains overall code integrity.
+ Added test cases for cumSum function to confirm correctness.
+ Verified that all tests pass successfully, confirming the correctness of the new cumulative sum functionality.
@jiyo4476 jiyo4476 changed the title [FEAT] Implement NDArray::cumprod Closes #46 [FEAT] Implement NDArray::cumprod and NDArray::cumsum Closes #45 & Closes #46 Sep 29, 2024
This was referenced Sep 29, 2024
Open
Open
@jiyo4476
Update cumulative axis freeing
ad36bbd 
+ Moved freeing pointers to different order to free sooner.
+ Tested to make sure functionality is correct after refactor.
@henrique-borba henrique-borba self-assigned this Sep 30, 2024
@henrique-borba henrique-borba self-requested a review September 30, 2024 20:03
@henrique-borba henrique-borba added the enhancement New feature or request label Sep 30, 2024
@jiyo4476
Refactor freeing NDArrays for better memory management
5029573 
+ Refactored the code in arithmetics.c to improve memory management by ensuring NDArrays are freed appropriately.
+ Ensured that all dynamically allocated NDArrays are freed to prevent memory leaks.
+ Verified that the function maintains its original functionality and correctness.
+ Confirmed that all tests pass successfully, ensuring the correctness of the changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@henrique-borba henrique-borba Awaiting requested review from henrique-borba

At least 1 approving review is required to merge this pull request.

Assignees

@henrique-borba henrique-borba

Labels
enhancement New feature or request
Projects
NumPower Extension
Status: In Progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jiyo4476 @henrique-borba