-
Notifications
You must be signed in to change notification settings - Fork 31
Hk from kalman gain storage #417
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
base: master
Are you sure you want to change the base?
Conversation
…g observations, add option to xml
…g observations, take offset into account, add logging and improve code
…g observations, take offset into account
…value for missing observations in Steady State filtering
…value for missing observations in Steady State filtering, move code to separate inner class
…ovation value for missing observations in Steady State filtering, move code to separate inner class" This reverts commit 896c649.
…value for missing observations in Steady State filtering, move code to separate inner class
…value for missing observations in Steady State filtering, move code to separate inner class
…ssociation/OpenDA into HKFromKalmanGainStorage
…rage # Conflicts: # core/java/src/org/openda/utils/io/KalmanGainStorage.java # core/java/test/org/openda/utils/io/KalmanGainStorageTest.java
…FromKalmanGainStorage # Conflicts: # algorithms/java/src/org/openda/algorithms/kalmanFilter/EnKF.java # core/java/src/org/openda/blackbox/wrapper/BBStochModelInstance.java # core/java/src/org/openda/interfaces/IModelInstance.java
nilsvanvelzen
left a comment
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.
Can you please squash you PR first because
- we do not want to have all these commits in main
- it makes reviewing very difficult
| for(int i=0;i<m;i++){ | ||
| ArrayList<Integer> missingObservationIndices = new ArrayList<>(); | ||
| ArrayList<Integer> availableObservationIndices = new ArrayList<>(); | ||
| for (int i = 0; i < gainStorageObservationIdsArray.length; i++) { |
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.
You mentioned that documentation will be part of the repository. I suggest to briefly explain what is going on + a link to the detailed description in the document.
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.
Will do
| // find matching column in steady-state gain | ||
| String gainVectorId = obsIds[i]+":"+Math.round(obsTimeOffsets[i]*24.0*3600.0); //conversion to seconds | ||
| Results.putProgression("processing obs "+gainVectorId+"\n"); | ||
| String gainVectorId = obsIds[i] + ":" + Math.round(obsTimeOffsets[i] * 24.0 * 3600.0); //conversion to seconds |
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 was part of the old code as well. But it puzzles me a bit what this stuff with conversion to seconds is here. You could add a line of comment to explain what kind of ID we are going to construct.
| private double[] readGainTime; | ||
| private int steadyStateTimeCounter = 0; | ||
| private double skipAssimilationStandardDeviationFactor = Double.POSITIVE_INFINITY; | ||
| private boolean tryCompensatingForMissingObservationWithHK = false; |
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 is a long name :-) What is HK in this context? H observation interpolation and K the gain? Not clear to me.
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'll ask Julius and Martin if they can give me a better description of HK for inspiration
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.
Suggestion: estimateMissingObservation
| </xs:element> | ||
| <xs:element name="tryCompensatingForMissingObservationWithHK" type="xs:boolean" minOccurs="0"> | ||
| <xs:annotation> | ||
| <xs:documentation>When this option is true AND in the kalman gain storage HK has been stored, for locations with missing observations the innovation value </xs:documentation> |
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.
again that "HK" is too cryptical for me (might find the answer later)
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'll ask Julius and Martin if they can give me a better description of HK for inspiration
|
I'm having difficulties squashing commits, I'll create a new branch and PR to see if that will resolve the issue. |
Implementation of calculating Kalman Gain for stations with missing observations.
In the document below the developments are explained with mathematical background and references to the code:
SSKF_compensate_for_missing_observations.docx