-
Notifications
You must be signed in to change notification settings - Fork 55
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
DTW method realization #517
base: dev
Are you sure you want to change the base?
Conversation
|
||
|
||
fun main() { | ||
with(DoubleField.bufferAlgebra.withSize(5)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Examples and tests must be moved to examples package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is I cannot run tests from exmaple Execution failed for task ':kmath-symja:compileKotlin'
, but can run from commonTest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please merge current dev branch. It is fixed there.
kmath-stat/src/commonMain/kotlin/space/kscience/kmath/series/DynamicTimeWarping.kt
Outdated
Show resolved
Hide resolved
kmath-stat/src/commonMain/kotlin/space/kscience/kmath/series/DynamicTimeWarping.kt
Outdated
Show resolved
Hide resolved
* There is special cases for i = 0 or j = 0. | ||
*/ | ||
|
||
public fun costMatrix(series1 : DoubleBuffer, series2 : DoubleBuffer) : DoubleBufferND { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it have to be public and top-level?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this into the body of DTW main method.
kmath-stat/src/commonMain/kotlin/space/kscience/kmath/series/DynamicTimeWarping.kt
Show resolved
Hide resolved
* for two series comparing and penalty for this alignment. | ||
*/ | ||
|
||
public fun dynamicTimeWarping(series1 : DoubleBuffer, series2 : DoubleBuffer) : DynamicTimeWarpingData { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method seems to be primary. But it pollutes the global name space. Maybe rename DynamicTimeWarpingData
to DynamicTimeWarping
and replace this method with a fake constructor (a function with name DynamicTimeWarping
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't understand exactly. I have to remove class DynamicTimeWarpingClass or change this method name?
kmath-stat/src/commonMain/kotlin/space/kscience/kmath/series/DynamicTimeWarping.kt
Outdated
Show resolved
Hide resolved
kmath-stat/src/commonMain/kotlin/space/kscience/kmath/series/DynamicTimeWarping.kt
Outdated
Show resolved
Hide resolved
kmath-stat/src/commonMain/kotlin/space/kscience/kmath/series/SomeTests.kt
Outdated
Show resolved
Hide resolved
kmath-stat/src/commonMain/kotlin/space/kscience/kmath/series/DynamicTimeWarping.kt
Outdated
Show resolved
Hide resolved
kmath-stat/src/commonMain/kotlin/space/kscience/kmath/series/DynamicTimeWarping.kt
Outdated
Show resolved
Hide resolved
id_x == 0 || costMatrix[id_x, id_y] == costMatrix[id_x, id_y - 1] + abs(series1[id_x] - series2[id_y]) -> { | ||
moveOption(DtwOffset.LEFT) | ||
} | ||
id_y == 0 || costMatrix[id_x, id_y] == costMatrix[id_x - 1, id_y] + abs(series1[id_x] - series2[id_y]) -> { | ||
moveOption(DtwOffset.BOTTOM) | ||
} | ||
costMatrix[id_x, id_y] == costMatrix[id_x - 1, id_y - 1] + abs(series1[id_x] - series2[id_y]) -> { | ||
moveOption(DtwOffset.DIAGONAL) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like equality test for double values. @SPC-code what do you think about it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late reply. Equality should not be tested for floating points unless it is magic constants like 0.0.
…eadability. Fix code style violations.
This is my first attempt to merge DTW method to kmath-stat series package. You can find short description in my code. Honestly, I'm doing merge request for the first time, so if I have missed rules of arrangement my apologize.