Alignment modernisation

First (and not final!) step of code modernization and cleanup:

• partial fix for #2: replaced private definition of matrix (AlMat) and vector (AlVec) by types from Eigen. Did not (yet) do this for the symmetric matrix (AlSymMat) as we rely heavily on it being symmetric and no drop-in replacement in Eigen exists. Removed dependence on LAPACK.
• sanitized subset of the solver tools. Changed default tool by one based on Eigen.
• added a script to run a set of solver tools on the same input data to allow for a comparison. Founds a bug in the Eigen and Gsl solver tools in doing so
• modernized the tools in TAlignment and AlignSolvTools to use 'extends' and Gaudi::Property

changed the description

@graven @bimitres @sborghi @smalde @lgrillo @rmatev FYI

changed the description

Thanks a lot for preparing this!

Perhaps the compilation problem can also be addressed here or in a separate issue.

The Eigen compilation warning is this:

/cvmfs/lhcb.cern.ch/lib/lcg/releases/LCG_97/eigen/3.3.7/x86_64-centos7-gcc9-opt/include/eigen3/Eigen/src/Core/Transpose.h:64:68: warning: implicitly-declared ‘Eigen::Block<const Eigen::Matrix<double, -1, -1>, 1, -1, false>::Block(const Eigen::Block<const Eigen::Matrix<double, -1, -1>, 1, -1, false>&)’ is deprecated [-Wdeprecated-copy]
   64 |     explicit inline Transpose(MatrixType& matrix) : m_matrix(matrix) {}

I don't know how to get rid of it.

added 1 commit

• d50cb050 - cleanup

Compare with previous version

added 2 commits

• d3c05355 - use some pragma magic to suppress deprecated-copy warning in Eigen
• 3e28eed1 - modernize

Compare with previous version

added 1 commit

• 36a1a6e5 - Fixed formatting

Compare with previous version

added 1 commit

• 863c138a - slight improvement in memory management using unique_ptr, but not really done yet

Compare with previous version

added 1 commit

• 769a348e - use ServiceHandle

Compare with previous version

added 1 commit

• b2d033a8 - Fixed formatting

Compare with previous version

mentioned in merge request !124 (merged)

unmarked as a Work In Progress

added cleanup modernisation labels

/ci-test --merge

• [2020-06-04 12:06] Validation started with lhcb-master-mr#885
• [2020-06-05 00:11] Automatic merge failed in lhcb-sanitizers#583
• [2020-06-06 00:20] Validation started with lhcb-gaudi-head#2623

added lhcb-sanitizers label

removed lhcb-sanitizers label

added lhcb-gaudi-head label

removed lhcb-gaudi-head label

@rosen Automatic merge failed in lhcb-sanitizer. Can you confirm that this is a problem with the slot? If so, I will include it in another nightly slot.

added lhcb-head label

added lhcb-gaudi-head label and removed lhcb-head label

assigned to @frodrigu

assigned to @jonrob and unassigned @frodrigu

removed lhcb-gaudi-head label

@jonrob the nightly appears to be fine.

@graven @wouter can you please conclude and when done resolve the discussions above.

mentioned in issue #2

I have another question: I just found that 'master' was never treated with lb-format, while run2-patches is. This explains like 99% of the difference between those two branches. Shall I include a full format of the project here?

added 2 commits

• ccc75457 - removed some spurious commented code
• c22a2220 - fixed counting of negative eigenvalues

Compare with previous version

added 1 commit

• aecc4a0c - Fixed formatting

Compare with previous version

mentioned in merge request !125 (merged)

added lhcb-gaudi-head lhcb-sanitizers labels

changed title from Alignmentmodernization may2020 to Alignment modernisation

mentioned in merge request !122 (merged)

@rmatev I just noticed I am not a maintainer here, so cannot merge. If I am supposed to as RTA maintainer could you give me the required access rights ?

I can approve the MR if everyone is in agreement. Shall I do so?

• [2020-06-10 00:12] Validation started with lhcb-sanitizers#590
• [2020-06-10 00:28] Validation started with lhcb-gaudi-head#2627
• [2020-06-10 04:07] Validation started with lhcb-sanitizers#591
• [2020-06-11 00:13] Validation started with lhcb-sanitizers#592
• [2020-06-11 00:19] Validation started with lhcb-gaudi-head#2628
• [2020-06-12 00:13] Validation started with lhcb-sanitizers#593
• [2020-06-12 00:20] Validation started with lhcb-gaudi-head#2629
• [2020-06-13 00:12] Validation started with lhcb-sanitizers#594
• [2020-06-13 00:20] Validation started with lhcb-gaudi-head#2630
• [2020-06-14 00:14] Validation started with lhcb-sanitizers#595
• [2020-06-14 00:16] Validation started with lhcb-gaudi-head#2631
• [2020-06-15 00:17] Validation started with lhcb-sanitizers#596
• [2020-06-15 00:24] Validation started with lhcb-gaudi-head#2632
• [2020-06-16 00:15] Validation started with lhcb-sanitizers#597
• [2020-06-16 00:22] Validation started with lhcb-gaudi-head#2633
• [2020-06-17 00:15] Validation started with lhcb-gaudi-head#2634
• [2020-06-17 00:16] Validation started with lhcb-sanitizers#598
• [2020-06-17 02:54] Validation started with lhcb-gaudi-head#2635
• [2020-06-18 02:15] Validation started with lhcb-sanitizers#599
• [2020-06-18 02:19] Validation started with lhcb-gaudi-head#2636

changed the description

@wouter @bimitres we've now four related MRs: this, the formatting !125 (merged) and the new tests !122 (merged) !124 (merged). I'd propose that I clean up and merge the tests and the formatting and then we can rebase this one. This way we'll have new (genuine) failures on master but hopefully the modernization here will fix them.

@wouter in practice, when I rebase this branch on top of the formatting, you'll have to do a reset on your local branch so it is important that you push any changes that you have locally before I do the rebase.

No worries: the remote branch is up to date with my local branch. I will then not do anything now.

I wouldn't merge !124 (merged) for now as it's not what we want for the moment, I need to add the iteration part there. For the other sounds good, let me know if I need to do anything there as I just put !122 (merged) on alignmentmodernizationMay2020 not on master.

no need to do anything on !122 (merged), I'll take care. Regarding !124 (merged), I know it's not quite what you want to have in the end but it's useful already to add the convergence check everywhere so I would merge it and open another MR for the test with different solvers. (or even add that here to this MR)

Ok that looks sensible, thanks for wrapping up all this.

added 32 commits

• aecc4a0c...c43dd5b1 - 4 commits from branch master
• 0aabd8e2 - Add .gitignore
• 9e0ee8a8 - Fix indentation
• 17f0cdb2 - Fix forward declarations
• 79865474 - Apply formatting
• 7fc55029 - save intermediate versions before final cleanup
• 3acbad95 - removed dependence on LAPACK. use Eigen for AlMat and AlVec
• 1923ae03 - moved to AlignTrTools
• 60999f19 - adapted to changes in AlignKernel. added Eigen solver
• 9d6f06b5 - added script to test solvers
• 7d399afb - adapted to change in solver tools
• 71b8b4db - adapted to changes in AlignKernel
• 2049a4dc - adapted to changes in AlignKernel
• 0edc3714 - make use of extends
• 94dd45e9 - make use of extends
• ebeeeff5 - make use of extends
• 52671185 - make use of extends
• e85bafb4 - make use of extends
• 1f50d1f2 - make use of extends
• 27b7d662 - - use extends
• d9444ac3 - - make use of extends
• e60afec0 - modernize use of Properties
• 7ae43e21 - cleanup
• 712464e8 - use some pragma magic to suppress deprecated-copy warning in Eigen
• d45dc455 - modernize
• 3beed7ac - slight improvement in memory management using unique_ptr, but not really done yet
• adba77d8 - use ServiceHandle
• 5891b820 - removed some spurious commented code
• a2f6e65d - fixed counting of negative eigenvalues

Compare with previous version

added 25 commits

• 109b78d5 - 1 commit from branch master
• 17ffe419 - save intermediate versions before final cleanup
• 20fb4038 - removed dependence on LAPACK. use Eigen for AlMat and AlVec
• ca2690ed - moved to AlignTrTools
• deabf840 - adapted to changes in AlignKernel. added Eigen solver
• 7bf668bb - added script to test solvers
• 940805ed - adapted to change in solver tools
• 7f689230 - adapted to changes in AlignKernel
• 118c77ba - adapted to changes in AlignKernel
• 50922c8f - make use of extends
• b7e50137 - make use of extends
• a26cc082 - make use of extends
• c73b2193 - make use of extends
• 9232610d - make use of extends
• 07813f60 - make use of extends
• 5a87ef93 - - use extends
• fdbd2295 - - make use of extends
• 62128910 - modernize use of Properties
• 832cffef - cleanup
• 7ad66f6a - use some pragma magic to suppress deprecated-copy warning in Eigen
• c0825f01 - modernize
• a269b2fc - slight improvement in memory management using unique_ptr, but not really done yet
• 13c9b846 - use ServiceHandle
• 832ae9fb - removed some spurious commented code
• 94b2d1ec - fixed counting of negative eigenvalues

Compare with previous version