search

iamlikeme / rainflow

Implementation of the rainflow-counting algorythm in Python
MIT License
105 stars 34 forks source link

Remaining one-half cycles is reversed? #17

Closed oysteoh closed 6 years ago

oysteoh commented 6 years ago

I'm currently implementing some analysis which are using the rainflow algorithm. I've tried a couple, and it seems all gives the same result. But - it seems like your code yields the remaining one half cycles in reversed order.. ? Or - it could be yours is the one which is correct, but then there is several others which gives wrong result... ( extract_cycles method )

Made a pull-request which proves and fix the error.

SaetreS commented 6 years ago

Do you have any test code and/or comparison results (and which other rfc?)? see also https://github.com/iamlikeme/rainflow/issues/4 with respect to end points (sounds like that might be the case for you).

oysteoh commented 6 years ago

Yes, i've added a pull-request with my ref-cases. Please take a look and check if it makes sense. If reverting rainflow.py and run tests you will find results from remaining halves to be reversed ( last 6 values )

iamlikeme commented 6 years ago

Hi @oysteoh, thank you for inspecting the code and submitting a pull request.

I don't really think that ordering of the cycles is significant to correcness. Note that full cycles are ordered by cycle end-time (they are yielded from the tail of the points deque), while half-cycles are ordered by cycle start-time (they are yielded from the head of the points deque). So the order is not chronological anyway.

That being said, I do not mind changing the order of the remaining cycles as you suggesed. I would appreciate, however, if you could write a simpler (minimal) test case, where maybe just the last two cycles change the order.

oysteoh commented 6 years ago

No problem, I will update my PR. Thanks!

oysteoh commented 6 years ago

Used the already existing case which is sufficient to prove my case. Is this ok? If not i will adjust :-)