Implementation of n-dimensional Gaussian constraints on fit parameters
Closes T214
Details
- Reviewers
tlatham - Maniphest Tasks
- T214: Multi-dimensional Gaussian constraints
- Commits
- rLAURAcf97b1614ff9: Implementation of n-dimensional Gaussian constraints
Tests with toys show spreads and correlations between parameters that are consistent with the constraints applied
Diff Detail
- Repository
- rLAURA laura
- Branch
- mwhitehe_nDconstraints
- Lint
No Lint Coverage - Unit
No Test Coverage - Build Status
Buildable 245 Build 245: arc lint + arc unit
Event Timeline
I've included the example of how to use the constraint - likely not the best place to have it in the end, but for now probably useful.
- Updates to move some functionality to LauFitObject
- Update code after move to LauFitObject and extend to LauSimFitCoordinator
Many thanks Mark, this looks great. Just a few small suggestions here and there.
inc/LauFitObject.hh | ||
---|---|---|
37–38 ↗ | (On Diff #397) | Do you need TSystem.h here? |
src/LauAbsFitModel.cc | ||
39 | Can drop this since it's already included in LauFitObject.hh | |
760–765 | No need for the if ( ! conVars_.empty() ). | |
774 | I wonder, can we have this TVectorD as a member of the StoreNDConstraints struct as well? It should be possible to construct it with the right size when the struct is first created in LauFitObject::addNDConstraint. | |
775 | To avoid copying the vector | |
942–944 | Can we call these something a bit more easily understandable? | |
951 | Better to push directly into itervec.conLauPars_ and avoid the need for the params vector. In fact, as currently written, I think that if you had more than one n-dimensional constraint you'd end up with too many parameters in all but the first one since params is declared outside all loops and is never cleared. | |
src/LauFitObject.cc | ||
31–32 ↗ | (On Diff #397) | |
97 ↗ | (On Diff #397) | Here you're relying on the fact that checkRepetition aborts the code. |
123 ↗ | (On Diff #397) | Same comment as above |
144–165 ↗ | (On Diff #397) | I would be inclined to allow the checks to continue so that you see all errors rather than one at a time. So you could have an allOK boolean that is initially true, then if an error is encountered it gets set to false. Then at the end of the function you just return that boolean. Perhaps also try to improve the loop variable names. |
src/LauSimFitCoordinator.cc | ||
788 ↗ | (On Diff #397) | Same comments on this function as for the corresponding one in LauAbsFitModel |
848 ↗ | (On Diff #397) | Same comments on this function as for the corresponding one in LauAbsFitModel |
Just a few trivial tidy-ups
inc/LauFitObject.hh | ||
---|---|---|
145 ↗ | (On Diff #399) | |
149 ↗ | (On Diff #399) | |
src/LauAbsFitModel.cc | ||
760–765 | To be picky, I would have used par singular here | |
791 | No need for this if, the loop will anyway not execute if it's empty | |
947 | ||
src/LauFitObject.cc | ||
150 ↗ | (On Diff #399) | |
175 ↗ | (On Diff #399) | |
src/LauSimFitCoordinator.cc | ||
779–782 ↗ | (On Diff #399) | |
789 ↗ | (On Diff #399) | Similarly, no need for the if here |
855 ↗ | (On Diff #399) |
Please also add an entry in the release notes, which should reference either this Differential revision or the corresponding Maniphest task