Transitionfix #3
Conversation
madsendennis
commented
Jun 4, 2021
madsendennis
commented
- Fixing the transition probability issue Error in transition probability computation #2.
- Updated to newest Scalismo version
- Updated the experiment parameters to adjust for the updated transition probability
- Added change check in logTransProb. - Removed double random implicit initialization.
There was a problem hiding this comment.
I checked the part that changed the calculation of the logTransitionProbability. This part looks reasonable to me. I only have there one minor question, but would not argue against merging even without discussing it.
I did not check all the changes needed to upgrade the dependency version as I guess this is handled sufficiently by the compiler. And regarding some parameter updates, there you are the expert how knows about the tiny little details of the experiments more than I do.
| val compensatedTo = to.copy(shapeParameters = ShapeParameters(from.shapeParameters.parameters + (to.shapeParameters.parameters - from.shapeParameters.parameters) / stepLength)) | ||
| val pdf = posterior.logpdf(compensatedTo.shapeParameters.parameters) | ||
| if (to.copy(shapeParameters = from.shapeParameters).allParameters != from.allParameters) { |
There was a problem hiding this comment.
Is there a problem when directly comparing the case classes ModelFittingParameters to each other? Or why do you use .allParameters?
There was a problem hiding this comment.
The ModelFittingParamater case class also has a "generatedBy" string in the constructor parameters which might be different. With .allParamaters we are sure only to compare the (shape, translation, rotation).
