search

laszukdawid / PyEMD

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

EEMD.py returning zero-valued IMFs - Problem with iterator being used more than once. #67

Closed rfmiotto closed 4 years ago

rfmiotto commented 4 years ago

Dear David,

Running the EEMD.py (that is, with __name__ == "__main__") and also the code in examples, I was obtaining zero-valued IMFs. I fixed this issue, and I want to share the solution with you.

I noticed that in EEMD.py the iterator all_IMFs was being used more than once, what would raise a StopIteration exception after its first use. Then, the code does not enter the loop to give values to E_IMF because all_IMFs is an empty list. There are 2 ways to fix this problem: (1) simply convert the iterator to a list before the next use, or (2) create two more iterators using itertools.tee.

If you want, I can send you the file or a push request (need permission). Thank you!

laszukdawid commented 4 years ago

Hi Renato,

As you might deduce from my late reply, there are other things for me that I'm focusing right now and I'm not sure when I'll do changes to PyEMD. Hopefully the weekend but...

I would like anyone to make any direct changes but I'm happy to review and merge any pull request. If you feel like writing one, that would help and speed things up :)

Thanks for highlighting the issue.

All the best, Dawid

rfmiotto commented 4 years ago

Hi Dawid,

Thank you for your reply. I will be pleased to help you by sending a push with the fix. However, the access permission to send push requests seems to be currently disabled for me. Could you please provide me access?

Best regards, Miotto

laszukdawid commented 4 years ago

Hi Miotto,

You shouldn't need to have edit access to my personal repository to propose a pull request. Changes are typically first done in the contributor's repository, i.e. create a fork and modify it, and then create a pull request. For small changes you should also be able, although I haven't tried it, to click "edit" button in the GitHub file view and on submission use option to do a pull request.

Forgive my reservation but before I grant anyone edit access I'd like them to contribute a bit so that I know they're contributors and not random people on the internet.

I have just made the change. Let me know if you think that's enough.

Thanks, Dawid

rfmiotto commented 4 years ago

Dear Dawid,

You are right!!! I just sent you the request now. I was trying to pull from the Atom text editor, and it was always complaining about the permission. I guess there might be some issue with the editor package then. Sorry I didn't try a different approach before. I got so used to always do stuff from Atom that I barely use git commands or access the website anymore.

About your change, it was missing the list also for the parallel case. I requested a pull in which I use iterators instead of list for the fix (I don't like the syntax because you need to create new iterators - so more variable names -, but probably it will be faster). I hope you find it useful and feel free to use your version if you prefer.

Thank you. Best regards, Miotto

laszukdawid commented 4 years ago

Guess with your recent change (#71) the issue is fixed. Thought that was for EEMD but guess these two pull requests were for two different files. I've extended your fix to EEMD (a3943316e).

Closing ticket.