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

Riccati Equation Solver #29

Merged
merged 2 commits into from Nov 13, 2017

Conversation

Projects
None yet
2 participants
@romgerale
Copy link
Contributor

romgerale commented Oct 28, 2017

Riccati Equation Solver, a proposal for the addition of an algebraic Riccati Equation Solver in the package org.hipparchus.linear.
It also incluses extensions in the Array2DRowRealMatrix to support Kronecker products and ComplexEigenDecomposition.

Riccati Equation Solver
Riccati Equation Solver, a proposal for the addition of an algebraic Riccati Equation Solver in the package org.hipparchus.linear.
It also incluses extensions in the Array2DRowRealMatrix to support Kronecker products and ComplexEigenDecomposition.
@maisonobe
Copy link
Contributor

maisonobe left a comment

This request seems OK to me, there are some minor glitches due to our style and naming conventions, but this we can fow ourselves after the merge. I have one change request that we cannot do ourselves, and also a legal request.
The change I would need would be to fix the header stating that you license this code to the "Hipparchus project" and not the "Apache software Foundation (ASF)". We are separate from the ASF and can only accept code for which we have a license to publish it.
The legal request is that since this is really a large contribution and you may continue to work with us maintaining it, we need you to file an Individual Contributor License Agreement (ICLA). See https://www.hipparchus.org/developers.html#Licensing_and_copyright for details.
You can send it to me (luc) directly, you already have my mail.
Once the headers haves been fixed and the ICLA is available, I will proceed with merging your PR and edit it to suit our conventions.

@romgerale

This comment has been minimized.

Copy link
Contributor

romgerale commented Nov 8, 2017

Done.

Riccati Equation Solver II
Fixing the headers.

@maisonobe maisonobe merged commit 2a6fc2a into Hipparchus-Math:master Nov 13, 2017

maisonobe added a commit that referenced this pull request Nov 13, 2017

@maisonobe

This comment has been minimized.

Copy link
Contributor

maisonobe commented Nov 13, 2017

I have merge the changes into the master branch.
Please take a look at what I have done to check I did do any mistake. I am not sure about the javadoc
parameter description I used.
In order to meet our development conventions, I have:

  • removed trailing blanks
  • removed tabs
  • replaced hard coded error messages with proper exceptions specifiers
  • completed all javadoc
  • fixed checkstyle errors
  • fixed findbugs errors

I am not sure the stack and unstack operations should really be in the Array2DRowRealMatrix class. They seem specific, are used only in the RiccatiEquationSolverImpl, and don't really need access
to private fields. Perhaps they should be private methods within RiccatiEquationSolverImpl.

@romgerale

This comment has been minimized.

Copy link
Contributor

romgerale commented Nov 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment