laszukdawid / PyEMD

Python implementation of Empirical Mode Decompoisition (EMD) method
https://pyemd.readthedocs.io/
Apache License 2.0
867 stars 224 forks source link

A possible way to solve issue#42 #44

Closed nescirem closed 5 years ago

nescirem commented 5 years ago

42

codecov-io commented 5 years ago

Codecov Report

Merging #44 into master will increase coverage by 0.77%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #44      +/-   ##
==========================================
+ Coverage    68.5%   69.28%   +0.77%     
==========================================
  Files          17       17              
  Lines        1810     1810              
  Branches      166      166              
==========================================
+ Hits         1240     1254      +14     
+ Misses        538      527      -11     
+ Partials       32       29       -3
Impacted Files Coverage Δ
PyEMD/CEEMDAN.py 94.26% <ø> (+4.91%) :arrow_up:
PyEMD/EMD.py 87.5% <0%> (+0.24%) :arrow_up:
PyEMD/tests/test_eemd.py 100% <0%> (+3.17%) :arrow_up:
PyEMD/EEMD.py 83.58% <0%> (+7.46%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update de373ec...2837477. Read the comment docs.

nescirem commented 5 years ago

Before: Figure_1.png After: Figure_2.png

laszukdawid commented 5 years ago

Thanks for proposing update to the code. Really appreciate it :)

Don't want to discourage you, but this modification is not introducing any significant changes to the code and it doesn't fix a problem. Not only it modifies exemplary usage, not the algorithm itself, but I'm also surprised that these modifications resulted in different outcome. If you are actually receiving better results after your changes then it might be because of either 1) different randomness seed in the second run, or 2) copying values casts them to a different floating point precision which introduces/removes noise.

Let me suggest a few changes to verify what these changes are doing: 1) Please use numpy's copy (np.copy) instead of copy which should copy exact object without modifying it. 2) Please set up the same seed on each execution. You can do so by using noise_seed method. For example

from PyEMD import CEEMDAN

S = generate_signal()  # provide here your signal

ceemdan = CEEMDAN()
ceemdan.noise_seed(12345)  # any numerical value
result = ceemdan(S)

This should help rule out some reasons why you are observing different results.

nescirem commented 5 years ago

Sorry for no detailed explanation and bad code before. But I do not think the ‘residuum' difference is caused by what you said. Try this code:

import pylab as plt
import copy
from PyEMD import CEEMDAN

max_imf = -1

# Signal options
N = 500
tMin, tMax = 0, 2*np.pi
T = np.linspace(tMin, tMax, N)

S = 3*np.sin(4*T)
Scopy = np.copy(S)

ErrorBefore = S-Scopy

ceemdan = CEEMDAN()
C_IMFs = ceemdan(S, T, max_imf)

ErrorAfter = S-Scopy

plt.ioff()
plt.subplot(2,1,1)
plt.plot(T, ErrorBefore, 'r')
plt.xlim((tMin, tMax))
plt.title("If copying values casts will intruduce/remove noise?")

plt.subplot(2,1,2)
plt.plot(T, ErrorAfter, 'r')
plt.xlim((tMin, tMax))
plt.title("No, CEEMDAN is the culprit!")

plt.show()

result: Figure_1.png You can find the value of S changed after "CEEMDAN it". So I try copy S to Scopy, python will send all the values from S to Scopy instead of share the memory address of S and Scopy.


Please abandon this pull request. according to the link , use S = S/scale_s instead of S[:] = S/scale_s in file CEEMDAN.py line 172. That will be help ;)

laszukdawid commented 5 years ago

Feel free to take a while and read again what I wrote in the previous comment. I wasn't addressing the problem but rather why your solution isn't a "solution".

Your updated example and suggestion makes more sense. It seems that the signal S is scaled inplace and not scaled back; only cimfs are.

Leftover of [:] is from refactoring; there used to be an array S_scaled that was updated few times and it was intentionally modified inplace.

Feel free to send another pull request with modified change. If you do so, could you also add a test, same as you just did, to make sure this mistake doesn't happen again?