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

added dimensionality test computed in slices #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mvonpapen
Copy link

would like to add a test that compares the dimensionality of neural activity as derived from covariance matrix in slices of Lslice length

Copy link
Collaborator

@rgutzen rgutzen left a comment

Choose a reason for hiding this comment

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

I think the new features of time slicing and calculating the dimensionality could be better used if they were implemented slightly differently:

Having a general time slicing in the sense of selecting a single slice might be useful for other tests as well, so the slice times could be implemented as an optional parameter directly in the generate_cc_matric in the correlation_test class. This should be always a single slice to not interfere the size and format of the output.
Since the eigenvalue_test inherits from this class the slicing feature is still available for this test. Tests like the dimensionality test who are based on multiple slices can iterate over slices and call the generate_cc_matrix for each slice.

For the dimensionality, it would be adequate to have a specific dimensionality_test class which inherits from the eigenvalue_test class. In this derived class the generate_prediction function would iterate over the defined slices, calling generate_prediction of its parent via super() in each iteration.

This way the the existing code would be reused, the eigenvalue_test would always return eigenvalues, and the dimensionality_test would always return an array of dimensionalities.

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.

2 participants