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

[Bug] CAD output is not the same as original MATLAB Version #443

Closed
Peyman7 opened this issue Jan 15, 2022 · 5 comments
Closed

[Bug] CAD output is not the same as original MATLAB Version #443

Peyman7 opened this issue Jan 15, 2022 · 5 comments
Labels
bug Indicates an unexpected problem or unintended behavior question

Comments

@Peyman7
Copy link

Peyman7 commented Jan 15, 2022

Describe the bug
CAD cell assembly detection method output is not the same as the original MATLAB version provided by the article's author. I have checked the source codes, and found the parameter alpha in the " # testing for higher order assemblies" part gives the wrong value as it should take the original alpha=0.05 in the right side.
alpha = alpha / float(len(w2_to_test) * n_as * (2 * max_lag + 1))
As alpha in previous lines recalled and its value is changed because of same alpha values in both side of equation, it should be changed in a way that the value of alpha in the right side always is be the same is the alpha input parameter.

To Reproduce
As alpha parameter name in input parameter may be changed to alph (as well as the same MATLAB version) and alpha parameter in the right side of alpha = alph/ float(len(w2_to_test) * n_as * (2 * max_lag + 1)) changes to alph.

Environment

  • OS: Windows
  • How you installed elephant (`pip):
  • Python version: 3.9.6
  • elephant python package version: 0.10.0

Environment

  • OS (e.g., Linux):
  • How you installed elephant (conda, pip, source):
  • Python version:
  • neo python package version:
  • elephant python package version:
  • (Any additional python package you want to include here)
@Moritz-Alexander-Kern
Copy link
Member

Moritz-Alexander-Kern commented Jan 17, 2022

Hey, thanks for your detailed report.
If I understand correctly this issue seems to be related to the name used for the variable/parameter producing unwanted behavior.
This needs some investigation, we will look into it.

@Moritz-Alexander-Kern Moritz-Alexander-Kern added question bug Indicates an unexpected problem or unintended behavior labels Feb 15, 2022
@Moritz-Alexander-Kern Moritz-Alexander-Kern added this to the v0.12.0 milestone Mar 31, 2022
@Moritz-Alexander-Kern
Copy link
Member

you've pointed to line:

alpha = alpha / float(len(w2_to_test) * n_as * (2 * max_lag + 1))

another change to alpha seems to occur here:

alpha = alpha * 2 / float(number_test_performed)

I suggest creating a validation test based on the original MATLAB implementation as a regression test.
Is this the MATLAB-implementation provided by the author you are using: https://github.com/DurstewitzLab/Cell-Assembly-Detection ?

@Peyman7
Copy link
Author

Peyman7 commented Jun 1, 2022

Yes, that link is the original MATLAB code published by the article's main author.

@Peyman7
Copy link
Author

Peyman7 commented Oct 11, 2022 via email

@Moritz-Alexander-Kern Moritz-Alexander-Kern linked a pull request Dec 2, 2022 that will close this issue
1 task
Moritz-Alexander-Kern added a commit that referenced this issue Jul 12, 2023
* keep input alpha as "alph" to be coherent with the MATLAB implementation of CAD

* replace np.int with int, since np.int is deprecated

* add comment on changes, reference to issue #443
@Moritz-Alexander-Kern Moritz-Alexander-Kern removed this from the v0.14.0 milestone Jul 20, 2023
@Moritz-Alexander-Kern
Copy link
Member

Fixed with #576

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior question
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants