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

Updated zscore for in-place operations #440

Merged
merged 3 commits into from
Dec 17, 2021

Conversation

kohlerca
Copy link
Collaborator

@kohlerca kohlerca commented Dec 12, 2021

The zscore function has a parameter inplace, to allow storing the z-scored values in the original AnalogSignal object(s).
The function always returns the AnalogSignal objects, whether or not an in-place operation is performed.

However, the in-place operation only operated at the level of the data array, i.e., whenever the NumPy array from the original AnalogSignal was modified with the z-scored values, a new AnalogSignal object was created pointing to that NumPy array. Therefore, this has never been a true in-place operation, as the returned objects differed from the inputs.

This PR fixes that by returning the original AnalogSignal when inplace is True, and a new object (also using a different data array) when inplace is False.

Moreover, the units in the original AnalogSignal object were not modified when inplace is True and only the returned object was dimensionless (see #384). Therefore, the original AnalogSignal ended up with z-scored values with a unit such as uV, which is misleading. The code in the PR now sets the unit to dimensionless in the original AnalogSignal.

Finally, the docstring mentioned that the in-place operation was not attempted if the AnalogSignal data type did not allow it (i.e., whenever the type was not float). The PR code now checks, for every object in the input list, whether the type allows the in-place operation. If inplace is True and this is not allowed, the parameter is ignored for that AnalogSignal (i.e., a copy will be returned along with a new AnalogSignal object), and a warning to the user is shown. Alternatively, this could be done before the main loop, checking if there is at least one object in the input list that does not allow the operation, and then setting inplace to False in that case (i.e., preventing any in-place operation at all).

To test the behavior of the updated function:

import numpy as np
import quantities as pq
import neo
from elephant.signal_processing import zscore

for inplace, dtype in ((True, np.int64), (False, np.int64),
                       (True, np.float64), (False, np.float64)):

    print("=======================")
    print(f"Inplace: {inplace}; dtype: {dtype}")

    test = neo.AnalogSignal(np.random.normal(10, 2, (60000, 96)),
                   units='uV', dtype=dtype, sampling_rate=30000*pq.Hz)

    zscored = zscore(test, inplace=inplace)

    print("IDs", id(test), id(zscored))
    print("Same object?", test is zscored)

    print(test[:1, :5])
    print(zscored[:1, :5])
    print("Equal data?", np.array_equal(test, zscored))

which produces

=======================
Inplace: True; dtype: <class 'numpy.int64'>
/home/koehler/PycharmProjects/elephant_fix/elephant/signal_processing.py:172: UserWarning: Could not perform inplace operation as the original signal dtype is not float. Source: None
  warnings.warn("Could not perform inplace operation as the "
IDs 139982366671472 139982321011504
Same object? False
[[11 10  8  8 10]] uV
[[ 0.73385822  0.25246159 -0.74701266 -0.74680096  0.24532251]] dimensionless
Equal data? False
=======================
Inplace: False; dtype: <class 'numpy.int64'>
IDs 139982321011840 139982321011728
Same object? False
[[13 12  8  5 10]] uV
[[ 1.74665356  1.23204397 -0.73947224 -2.22643084  0.24853166]] dimensionless
Equal data? False
=======================
Inplace: True; dtype: <class 'numpy.float64'>
IDs 139982321011616 139982321011616
Same object? True
[[ 0.4987234  -0.25468458 -1.32321378  0.41974894  0.32817535]] dimensionless
[[ 0.4987234  -0.25468458 -1.32321378  0.41974894  0.32817535]] dimensionless
Equal data? True
=======================
Inplace: False; dtype: <class 'numpy.float64'>
IDs 139982333189472 139982321011840
Same object? False
[[11.18127483  8.5491104  11.8618302  13.54534356 11.8022081 ]] uV
[[ 0.58965413 -0.73046345  0.93888403  1.77151628  0.89908117]] dimensionless
Equal data? False

…n-place operations, and also to return the original object instead of a new AnalogSignal object pointing to the original array
@pep8speaks
Copy link

pep8speaks commented Dec 12, 2021

Hello @kohlerca! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-12-13 10:59:19 UTC

@coveralls
Copy link
Collaborator

coveralls commented Dec 12, 2021

Coverage Status

Coverage increased (+0.01%) to 88.689% when pulling d454a50 on INM-6:fix/zscore into 0c54079 on NeuralEnsemble:master.

Cristiano Köhler added 2 commits December 13, 2021 11:29
…place operation in zscore.

Changed test_zscore_single_inplace_int as the function now raises ValueError.
@kohlerca
Copy link
Collaborator Author

As discussed, the function was modified to raise an error when the in-place operation is not valid. Also, the duplicate is made using the Neo function.

All unit tests were modified to check if the objects are the same/different according to the inplace parameter. Moreover, the unit test using an AnalogSignal of type int as input with inplace=True was modified to check for the exception.

@Moritz-Alexander-Kern
Copy link
Member

after modification to raise an error, check behavior with added exception handling:

import numpy as np
import quantities as pq
import neo
from elephant.signal_processing import zscore

for inplace, dtype in ((True, np.int64), (False, np.int64),
                       (True, np.float64), (False, np.float64)):

    print("=======================")
    print(f"Inplace: {inplace}; dtype: {dtype}")

    test = neo.AnalogSignal(np.random.normal(10, 2, (60000, 96)),
                   units='uV', dtype=dtype, sampling_rate=30000*pq.Hz)
    try:
        zscored = zscore(test, inplace=inplace)

        print("IDs", id(test), id(zscored))
        print("Same object?", test is zscored)

        print(test[:1, :5])
        print(zscored[:1, :5])
        print("Equal data?", np.array_equal(test, zscored))
    except:
        print("error was raised")

which produces the same output as in the first comment by @kohlerca, except now an error is raised with Inplace: True; dtype: <class 'numpy.int64'>, as expected.

=======================
Inplace: True; dtype: <class 'numpy.int64'>
error was raised
=======================
Inplace: False; dtype: <class 'numpy.int64'>
IDs 139652275362384 139652275359920
Same object? False
[[ 8  6  9  7 12]] uV
[[-0.74279302 -1.73908209 -0.24362649 -1.23294208  1.24192959]] dimensionless
Equal data? False
=======================
Inplace: True; dtype: <class 'numpy.float64'>
IDs 139652275363168 139652275363168
Same object? True
[[ 1.30684587  0.06441688 -1.34509361 -0.47104841  0.33951849]] dimensionless
[[ 1.30684587  0.06441688 -1.34509361 -0.47104841  0.33951849]] dimensionless
Equal data? True
=======================
Inplace: False; dtype: <class 'numpy.float64'>
IDs 139652275362160 139652275362384
Same object? False
[[ 6.62071703  8.79765984 11.69532766 14.2552753  13.66768817]] uV
[[-1.68625204 -0.59526308  0.8431924   2.12242394  1.83686467]] dimensionless
Equal data? False

@Moritz-Alexander-Kern Moritz-Alexander-Kern merged commit cfdecd8 into NeuralEnsemble:master Dec 17, 2021
@Moritz-Alexander-Kern Moritz-Alexander-Kern mentioned this pull request Mar 9, 2022
1 task
@Moritz-Alexander-Kern Moritz-Alexander-Kern deleted the fix/zscore branch October 28, 2022 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fix for an indentified bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] zscore(..., inplace=True) does not change the units of the original signal to dimensionless
4 participants