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

Add new numerical methods to solve linear system: #531

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

VarenytsiaMykhailo
Copy link

  1. Thomas method (прогонки)
  2. Jacobi method (iterative)
  3. Seidel method (iterative, improvement version of the Jacobi method)

and unit tests.

1. Thomas method (прогонки)
2. Jacobi method (iterative)
3. Seidel method (iterative, improvement version of the Jacobi method)
Copy link
Contributor

@SPC-code SPC-code left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Methods should use LinearAlgebra and generalized for non-Float64 types.

Also, please add an example of usage in example module and a short markdown documentation on when those methods should be used.

* @return vector X - solution of the system 'A*X=B'.
*/
@UnstableKMathAPI
public fun solveSystemByJacobiMethod(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Global namespace polution. The method must be made an extension for something. Using Double-only methods is not a KMath way, one should use Field or Ring if one does not need division for that.

private var cachedMachineEpsilonPrecision: Double? = null

@UnstableKMathAPI
public val machineEpsilonPrecision: Double
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Global namespace polution. And it should be Algebra attribute.

}
}

var X: Point<Double> = initialApproximation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code style violation

return X
}

private fun calcNorm(x1: Point<Double>, x2: Point<Double>): Double {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function could be moved inside the only function it is used in


norm = calcNorm(X, xTmp)
X = xTmp
} while (norm > epsilonPrecision)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it be infinite?

}
}

val X: MutableStructure1D<Double> = initialApproximation?.let {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code style violation.

return X
}

private fun calcNorm(x1: Point<Double>, x2: Point<Double>): Double {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function duplicate

* @return vector X - solution of the system 'A*X=B'.
*/
@UnstableKMathAPI
public fun solveSystemByThomasMethod(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Global namespace pollution and unnecessary specialization. The same as above.

)

assertEquals(4, result.size)
val absoluteTolerance = 0.00000000000001
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a test method for structure equality

*/
@Test
fun exceptionTest1() =
Double.algebra.linearSpace.run {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like with more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants