-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add converted high dimensional covariance testing #546
Add converted high dimensional covariance testing #546
Conversation
Added initial attempts for converted two sample test functions for covariance matrices, along with tests and a data file to compare original outputs with
Implement Python versions of Two-Sample Test and HC2018 Hypothesis Tests from R Code, and Add Tests to Ensure Correctness
WalkthroughThe new files Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TestSuite
participant HypothesisModule
User->>TestSuite: Run tests
TestSuite->>HypothesisModule: Import statistical functions
TestSuite->>HypothesisModule: Apply sy2010 test
HypothesisModule-->>TestSuite: Return results
TestSuite->>HypothesisModule: Apply clx2013 test
HypothesisModule-->>TestSuite: Return results
TestSuite->>HypothesisModule: Apply hc2018 test
HypothesisModule-->>TestSuite: Return results
TestSuite->>User: Output test results
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (6)
python/python/bystro/covariance/hypothesis_hd.py (6)
56-72
: Ensure input validation and edge case handling.The function
cov_eigs
currently lacks input validation. It is recommended to add checks to handle edge cases and invalid inputs, such as empty arrays or non-numeric data, to prevent potential issues.
- File:
python/python/bystro/covariance/hypothesis_hd.py
- Lines: 56-72
Analysis chain
Ensure input validation and edge case handling.
The function
cov_eigs
calculates the eigenvalues correctly. However, ensure that the input matrix X is validated properly to avoid potential issues.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the input validation for `cov_eigs`. # Test: Check if the function handles invalid inputs correctly. python -c " import numpy as np from bystro.covariance.hypothesis_hd import cov_eigs try: cov_eigs(np.array([])) except ValueError as e: assert str(e) == 'Input matrix is empty!', 'Unexpected error message!' print('Input validation works correctly.') "Length of output: 353
Script:
#!/bin/bash # Search for input validation logic within the `cov_eigs` function in hypothesis_hd.py ast-grep --lang python --pattern $'def cov_eigs($_):\n $$$\n if $_:\n raise ValueError($_)\n $$$' hypothesis_hd.pyLength of output: 191
Script:
#!/bin/bash # Locate the correct file path for hypothesis_hd.py fd hypothesis_hd.py # Once the file is located, search for input validation logic within the `cov_eigs` function ast-grep --lang python --pattern $'def cov_eigs($_):\n $$$\n if $_:\n raise ValueError($_)\n $$$' $(fd hypothesis_hd.py)Length of output: 373
336-393
: Ensure input validation and edge case handling.The function
calibration
lacks input validation for its parameters. To avoid potential issues, consider adding checks to ensure thatn1
,n2
,p
,n
,alpha
,const
,iterations
,K
, andseed
are within acceptable ranges.
- Add validation to ensure
n1
,n2
,p
,n
,iterations
, andK
are positive integers.- Ensure
alpha
andconst
are within the range (0, 1).- Validate that
seed
is a non-negative integer.Analysis chain
Ensure input validation and edge case handling.
The function
calibration
performs calibration correctly. However, ensure that the inputs n1, n2, p, n, and other parameters are validated properly to avoid potential issues.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the input validation for `calibration`. # Test: Check if the function handles invalid inputs correctly. python -c " from bystro.covariance.hypothesis_hd import calibration try: calibration(0, 0, 0, 0) except ValueError as e: assert str(e) == 'Invalid input values!', 'Unexpected error message!' print('Input validation works correctly.') "Length of output: 337
Script:
#!/bin/bash # Search for the `calibration` function implementation in the codebase. ast-grep --lang python --pattern $'def calibration($_, $_, $_, $_, $_=0.05, $_=0.5, $_=100, $_=100, $_=2021) -> $_: $$$' --jsonLength of output: 135
Script:
#!/bin/bash # Search for the `calibration` function by name and include context lines for manual inspection. rg 'def calibration' -A 20Length of output: 1441
Script:
#!/bin/bash # Search for the `calibration` function by name and include more context lines to capture the full implementation. rg 'def calibration' -A 60Length of output: 4481
96-117
: Ensure input validation and edge case handling.The function
t_func
does not currently validate its inputs. To avoid potential issues, it is recommended to add validation checks forlambda_
,gamma
, andeta0
to ensure they are of the expected types and values.
- Add checks to ensure
lambda_
is a non-emptynp.ndarray
.- Validate that
gamma
andeta0
are floats and notNone
.Analysis chain
Ensure input validation and edge case handling.
The function
t_func
calculates the T value correctly. However, ensure that the inputs lambda_, gamma, and eta0 are validated properly to avoid potential issues.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the input validation for `t_func`. # Test: Check if the function handles invalid inputs correctly. python -c " import numpy as np from bystro.covariance.hypothesis_hd import t_func try: t_func(np.array([]), None, None) except TypeError as e: assert str(e) == 'Invalid input values!', 'Unexpected error message!' print('Input validation works correctly.') "Length of output: 359
Script:
#!/bin/bash # Search for the implementation of the `t_func` function to check for input validation ast-grep --lang python --pattern $'def t_func(lambda_: np.ndarray, gamma: float, eta0: float) -> float: $$$ ' --debugLength of output: 340
Script:
#!/bin/bash # Search for the implementation of the `t_func` function to check for input validation ast-grep --lang python --pattern $'def t_func(lambda_: np.ndarray, gamma: float, eta0: float) -> float: $$$ 'Length of output: 1773
Script:
#!/bin/bash # Search for input validation logic within the `python/python/bystro/covariance/hypothesis_hd.py` file rg "if .*lambda_.*|if .*gamma.*|if .*eta0.*" ./python/python/bystro/covariance/hypothesis_hd.py -A 3Length of output: 249
157-230
: Ensure input validation and edge case handling.The function
two_sample_test_
performs a two-sample test correctly. However, it lacks input validation for parameters such asX
,Y
, andn
. Ensure that these inputs are validated properly to avoid potential issues.
- Validate that
X
andY
are non-empty numpy arrays.- Validate that
n
is a positive integer.- Consider adding checks for other parameters to ensure they meet expected conditions.
Analysis chain
Ensure input validation and edge case handling.
The function
two_sample_test_
performs a two-sample test correctly. However, ensure that the inputs X, Y, n, and other parameters are validated properly to avoid potential issues.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the input validation for `two_sample_test_`. # Test: Check if the function handles invalid inputs correctly. python -c " import numpy as np from bystro.covariance.hypothesis_hd import two_sample_test_ try: two_sample_test_(np.array([]), np.array([]), 10) except ValueError as e: assert str(e) == 'Invalid input values!', 'Unexpected error message!' print('Input validation works correctly.') "Length of output: 386
Script:
#!/bin/bash # Description: Search for input validation logic within the `two_sample_test_` function. ast-grep --lang python --pattern $'def two_sample_test_($_, $_, $_, $_, $_, $_, $_, $_, $_, $_):\n $$$'Length of output: 106
Script:
#!/bin/bash # Description: Search for common input validation patterns within the `two_sample_test_` function. rg -A 20 'def two_sample_test_' --glob '*.py'Length of output: 1495
Script:
#!/bin/bash # Description: Search for input validation logic within the entire function body of `two_sample_test_`. rg -A 50 'def two_sample_test_' --glob '*.py'Length of output: 3891
75-93
: Ensure input validation and edge case handling.The function
k_func
implements a kernel function correctly but does not include input validation for the parameterx
. It is recommended to add input validation to ensurex
is a float to avoid potential issues.
- Add a check at the beginning of the function to verify that
x
is a float. If not, raise aTypeError
with an appropriate message.Analysis chain
Ensure input validation and edge case handling.
The function
k_func
implements a kernel function correctly. However, ensure that the input value x is validated properly to avoid potential issues.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the input validation for `k_func`. # Test: Check if the function handles invalid inputs correctly. python -c " from bystro.covariance.hypothesis_hd import k_func try: k_func(None) except TypeError as e: assert str(e) == 'Input value must be a float!', 'Unexpected error message!' print('Input validation works correctly.') "Length of output: 327
Script:
#!/bin/bash # Search for the implementation of the `k_func` function to inspect for input validation logic. ast-grep --lang python --pattern $'def k_func($_):\n $$$' > k_func_implementation.txt # Display the content of the found implementation for manual inspection. cat k_func_implementation.txtLength of output: 92
Script:
#!/bin/bash # List all functions in the file to manually inspect the `k_func` function. ast-grep --lang python --pattern $'def $_($_):\n $$$' python/python/bystro/covariance/hypothesis_hd.pyLength of output: 105
Script:
#!/bin/bash # Search for the `k_func` function definition and display surrounding lines for context. rg 'def k_func' -A 20 python/python/bystro/covariance/hypothesis_hd.pyLength of output: 438
463-482
: Ensure input validation and edge case handling.The function
find_stable
does not currently validate the input listxs
for edge cases such as an empty list. To avoid potential issues, consider adding input validation to handle such cases.
- File:
python/python/bystro/covariance/hypothesis_hd.py
- Lines: 463-482
Analysis chain
Ensure input validation and edge case handling.
The function
find_stable
finds the stable point correctly. However, ensure that the input list xs is validated properly to avoid potential issues.
To verify the input validation for the
find_stable
function, we will inspect the code directly for any input validation logic.
To verify the input validation for the
find_stable
function, we will search for the specific pattern of input validation directly within the function.
To verify the input validation for the
find_stable
function, we will first locate the function and then search for input validation logic within it.
To verify the input validation for the
find_stable
function, we will first locate the function usingrg
and then manually inspect the lines around it for input validation logic.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the input validation for `find_stable`. # Test: Check if the function handles invalid inputs correctly. python -c " from bystro.covariance.hypothesis_hd import find_stable try: find_stable([]) except ValueError as e: assert str(e) == 'Input list is empty!', 'Unexpected error message!' print('Input validation works correctly.') "Length of output: 328
Script:
#!/bin/bash # Description: Check for input validation in the `find_stable` function. # Locate the `find_stable` function and check for input validation logic. ast-grep --lang python --pattern $'def find_stable(xs: List[float]) -> int { $$$ }' --search-inside $'if len(xs) == 0: raise ValueError("Input list is empty!")'Length of output: 385
Script:
#!/bin/bash # Description: Check for input validation in the `find_stable` function. # Locate the `find_stable` function and check for input validation logic. ast-grep --lang python --pattern $'def find_stable(xs: List[float]) -> int { $$$ if len(xs) == 0: raise ValueError("Input list is empty!") $$$ }'Length of output: 501
Script:
#!/bin/bash # Description: Locate the `find_stable` function and check for input validation logic. # Locate the `find_stable` function. ast-grep --lang python --pattern $'def find_stable(xs: List[float]) -> int { $$$ }' --json | jq '.matches[].range' # Check for input validation logic within the located function. ast-grep --lang python --pattern $'if len(xs) == 0: raise ValueError("Input list is empty!")'Length of output: 626
Script:
#!/bin/bash # Description: Locate the `find_stable` function and check for input validation logic. # Locate the `find_stable` function. rg 'def find_stable' -A 20Length of output: 1604
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
python/python/bystro/covariance/tests/hypothesis_hd_data.pkl
is excluded by!**/*.pkl
Files selected for processing (2)
- python/python/bystro/covariance/hypothesis_hd.py (1 hunks)
- python/python/bystro/covariance/tests/test_hypothesis_hd.py (1 hunks)
Additional context used
Ruff
python/python/bystro/covariance/hypothesis_hd.py
147-154: Return the negated condition directly
Inline condition
(SIM103)
Additional comments not posted (7)
python/python/bystro/covariance/tests/test_hypothesis_hd.py (4)
28-42
: Ensure the test data is consistent and correct.The test function
test_clx2013
correctly loads data and asserts the results. However, ensure that the data inhypothesis_hd_data.pkl
is consistent and correct for reliable tests.
45-110
: Ensure the test data is consistent and correct.The test function
test_hc2018
correctly loads data and asserts the results. However, ensure that the data inhypothesis_hd_data.pkl
is consistent and correct for reliable tests.
11-25
: Ensure the test data is consistent and correct.The test function
test_sy2010
correctly loads data and asserts the results. However, ensure that the data inhypothesis_hd_data.pkl
is consistent and correct for reliable tests.
113-142
: Ensure the test data is consistent and correct.The test function
test_two_sample_test
correctly loads data and asserts the results. However, ensure that the data inhypothesis_hd_data.pkl
is consistent and correct for reliable tests.python/python/bystro/covariance/hypothesis_hd.py (3)
233-333
: Ensure input validation and edge case handling.The function
two_sample_test
performs a two-sample test multiple times correctly. However, ensure that the inputs X, Y, n, k, and other parameters are validated properly to avoid potential issues.
396-461
: Ensure input validation and edge case handling.The function
c_tuning
tunes the constant C correctly. However, ensure that the inputs X, Y, n, and other parameters are validated properly to avoid potential issues.
9-53
: Ensure input validation and edge case handling.The function
two_way_sampling
performs two-way sampling correctly. However, ensure that the inputs X and Y are validated properly to avoid potential issues.Verification successful
Ensure input validation and edge case handling.
The function
two_way_sampling
includes input validation to handle edge cases. It raises aValueError
if the sample sizen
is greater than the minimum of half the maximum ofn1
andn2
,n1
, andn2
. This ensures that invalid sample sizes are appropriately handled.
- The input validation logic is present and correctly implemented in the
two_way_sampling
function.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the input validation for `two_way_sampling`. # Test: Check if the function handles invalid inputs correctly. python -c " import numpy as np from bystro.covariance.hypothesis_hd import two_way_sampling try: two_way_sampling(np.array([]), np.array([]), 10) except ValueError as e: assert str(e) == 'Invalid sample sizes!', 'Unexpected error message!' print('Input validation works correctly.') "Length of output: 386
Script:
#!/bin/bash # Description: Verify the input validation for `two_way_sampling`. # Test: Check if the function handles invalid inputs correctly. python3 -c " import numpy as np from bystro.covariance.hypothesis_hd import two_way_sampling try: two_way_sampling(np.array([]), np.array([]), 10) except ValueError as e: assert str(e) == 'Invalid sample sizes!', 'Unexpected error message!' print('Input validation works correctly.') "Length of output: 388
Script:
#!/bin/bash # Description: Verify the input validation logic in the `two_way_sampling` function. # Use ast-grep to find the `two_way_sampling` function and check for input validation logic. ast-grep --lang python --pattern $'def two_way_sampling($_, $_, $_, $_=2021):\n $$$' | grep -A 20 "def two_way_sampling"Length of output: 123
Script:
#!/bin/bash # Description: Verify the input validation logic in the `two_way_sampling` function. # Use ast-grep to find the `two_way_sampling` function and check for input validation logic. ast-grep --lang python --pattern $'def two_way_sampling($_, $_, $_, $_=2021):\n $$$' | grep -A 50 "def two_way_sampling"Length of output: 123
Script:
#!/bin/bash # Description: Verify the input validation logic in the `two_way_sampling` function. # Use rg to find the `two_way_sampling` function and extract its content. rg 'def two_way_sampling' -A 50Length of output: 3832
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
python/python/bystro/covariance/hypothesis_hd.py (1)
33-34
: Improve the error message for invalid sample sizes.The current error message "Invalid sample sizes!" is not descriptive enough. It would be better to provide more context about the error.
- raise ValueError("Invalid sample sizes!") + raise ValueError(f"Invalid sample size: n should be <= min(max(n1, n2) / 2, n1, n2), but got n={n}, n1={n1}, n2={n2}.")
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- python/python/bystro/covariance/hypothesis_hd.py (1 hunks)
Additional context used
Ruff
python/python/bystro/covariance/hypothesis_hd.py
120-120: Undefined name
lambda1
(F821)
120-120: Undefined name
lambda1
(F821)
121-121: Undefined name
lambda2
(F821)
121-121: Undefined name
lambda2
(F821)
123-123: Undefined name
lambda1
(F821)
123-123: Undefined name
lambda1
(F821)
123-123: Undefined name
epsilon
(F821)
124-124: Undefined name
lambda2
(F821)
124-124: Undefined name
lambda2
(F821)
124-124: Undefined name
epsilon
(F821)
175-175: Undefined name
check_efficient
(F821)
Additional comments not posted (14)
python/python/bystro/covariance/hypothesis_hd.py (14)
56-72
: LGTM!The function correctly calculates the eigenvalues of the sample covariance matrix.
75-93
: LGTM!The kernel function is correctly implemented.
96-117
: LGTM!The function correctly computes the T value using the kernel function.
204-304
: LGTM!The function correctly performs a two-sample test multiple times and includes calibration.
307-364
: LGTM!The function correctly performs calibration to find the threshold.
367-431
: LGTM!The function correctly tunes the constant C.
434-453
: LGTM!The function correctly finds the stable point in a list of rates.
456-474
: LGTM!The function correctly computes the moving variance of a list.
477-517
: LGTM!The function correctly performs the CLX2013 test.
520-614
: LGTM!The function correctly performs the SY2010 test.
617-757
: LGTM!The function correctly performs the HC2018 test.
760-778
: LGTM!The function correctly estimates the proportion of true null hypotheses.
781-801
: LGTM!The function correctly prints the rejected hypotheses.
803-848
: LGTM!The function correctly performs the adaptive step-up procedure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- python/python/bystro/covariance/tests/test_hypothesis_hd.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- python/python/bystro/covariance/tests/test_hypothesis_hd.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- python/python/bystro/covariance/hypothesis_hd.py (1 hunks)
- python/python/bystro/covariance/tests/test_hypothesis_hd.py (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- python/python/bystro/covariance/hypothesis_hd.py
- python/python/bystro/covariance/tests/test_hypothesis_hd.py
rates = [] | ||
|
||
if thres is None: | ||
thres = 2.6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we set this default? Could we logger.warn("Threshold thres
not specified, setting %d", 2.6)?
if not check_efficient(gamma, eig_xs, eig_ys, epsilon): | ||
return {"efficient": False, "c": 1} | ||
|
||
eta0 = np.std(eig_zs, ddof=1) * const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we also validate that const, alpha, epsilon, thres are in valid ranges? Typically we have been raising ValueError if an input is incorrect (e.g., r("Invalid input: const must be a float that is greater than x")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- python/python/bystro/covariance/hypothesis_hd.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- python/python/bystro/covariance/hypothesis_hd.py
OK by me! |
LGTM |
Added initial attempts for converted two sample test functions for covariance matrices, along with tests and a data file to compare original outputs with
Summary by CodeRabbit