Follow-up from "prototype of new alignment configuration"

The following discussions from !203 (merged) should be addressed:

• @sstahl started a discussion:

  Not for this MR but for an issue, we should fix that the PV reco also returns a dict of all track types. Then it should also not be necessary to specify two times that PatPV3DFuture is used and the option usePatPVFuture can be removed.

• @sstahl started a discussion:

  I think I would move most imports to the top of the file. It makes it harder to read the code in getAlignmentTracksAndPVs.

• @sstahl started a discussion:

  The function seems a bit useless. The only use seems to be to define some defaults. Are they different from the C++? Note one can always bind to AlignAlgorithm directly to change the properties. Maybe it makes sense to remove some of them to put the focus on the important ones which are used.

• @sstahl started a discussion:

  Either now or in a future MR, it would probably make sense to move this function to python/Humboldt.

• @sstahl started a discussion:

  We should find a way to change the default of this property globally. It probably always never makes sense in the Alignment.

• @sstahl started a discussion:

  Instead of make_hlt2_tracks.bind(light_reco=False) it might be better to use reconstruction.bind(make_reconstruction=make_default_reconstruction) * . This should protect you better that nothing changes, if this is desired. It could make it harder to switch to the PrKalman though.

  Alternatively, can you add a comment why you specify make_hlt2_tracks.bind(light_reco=False) and why this is necessary for the alignment.

  • I realize now that the name "default reconstruction" is not great.

• @sstahl started a discussion:

  Seems more logical to me to pass the best_tracks into this function than the whole reconstruction since you anyhow get the best_tracks above. This makes the function also more reusable.

• @sstahl started a discussion:

  Is it necessary to use "from Configurables" here? What does SurveyConstraints do? It should probably be marked as a TODO to fix this.

• @sstahl started a discussion:

  Does the alignment produce no output which enters the monitoring? I find it a bit strange that the monitors are defined independently of the alignment. I would have imagined that one needs to run the tracking at least twice, before and after the alignment. But I am ignorant to how this is setup now.

  Would it make a difference if the monitors run before the alignment?

  My feeling is that once the alignment follows the rules of functional algorithms, this function needs to be split in several parts. So it might better that the monitoring is not added here.

• @sstahl started a discussion: (+5 comments)

  I had another look at the MR as @rmatev asked me. It looks good to me as an intermediate step and to get tests in. Most comments I made can be probably added to issues. It might be good to clean up the description of the MR and associated issues.

  One thing I wonder is if running the alignment algorithms still changes tracks in place and doesn't produce a new container? I also mention this question in one comment.

• @sstahl started a discussion: (+2 comments)

  This test fails for me locally with the following stderror message:

  cat: Iter?/alignlog.txt: No such file or directory
  cat: Iter1?/alignlog.txt: No such file or directory
  cat: Iter2?/alignlog.txt: No such file or directory

  It seems to be different from what is in the nightlies.

  Can it be fixed or should it be disabled?