fix: protect TrackSelector against eta=nan

[timadye]

Protect against η=nan in Acts::TrackSelector::isValidTrack(), which would throw std::invalid_argument{"Eta is outside the abs eta bin edges"} exception. This can come about if somehow the track θ<0 or θ>π. We should try to prevent these out-of-range values, but having them shouldn't raise an exception.

[github-actions]

📊: Physics performance monitoring for 42b5486

Full contents

physmon summary

• ✅ Particles fatras
• ✅ Particles geant4
• ✅ Particles ttbar
• ✅ Vertices ttbar
• ✅ Truth tracking (KF)
• ✅ Truth tracking (GSF)
• ✅ Truth tracking (GX2F)
• ✅ Truth tracking (KF refit)
• ✅ Truth tracking (GSF refit)
• ✅ CKF finding performance | trackfinding | single muon | truth smeared seeding
• ✅ CKF fitting performance | trackfinding | single muon | truth smeared seeding
• ✅ CKF track summary | trackfinding | single muon | truth smeared seeding
• ✅ Seeding trackfinding | single muon | truth estimated seeding
• ✅ CKF finding performance | trackfinding | single muon | truth estimated seeding
• ✅ CKF fitting performance | trackfinding | single muon | truth estimated seeding
• ✅ CKF track summary | trackfinding | single muon | truth estimated seeding
• ✅ Seeding trackfinding | single muon | default seeding
• ✅ CKF finding performance | trackfinding | single muon | default seeding
• ✅ CKF fitting performance | trackfinding | single muon | default seeding
• ✅ CKF track summary | trackfinding | single muon | default seeding
• ✅ Seeding trackfinding | single muon | orthogonal seeding
• ✅ CKF finding performance | trackfinding | single muon | orthogonal seeding
• ✅ CKF fitting performance | trackfinding | single muon | orthogonal seeding
• ✅ CKF track summary | trackfinding | single muon | orthogonal seeding
• ✅ Seeding trackfinding | 4 muon x 50 vertices | default seeding
• ✅ CKF finding performance | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ CKF fitting performance | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ CKF track summary | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ Ambisolver finding performance | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ IVF notime | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ AMVF gauss notime | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ AMVF grid time | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ Seeding trackfinding | ttbar with 200 pileup | default seeding
• ✅ CKF finding performance | trackfinding | ttbar with 200 pileup | default seeding
• ✅ CKF fitting performance | trackfinding | ttbar with 200 pileup | default seeding
• ✅ CKF track summary | trackfinding | ttbar with 200 pileup | default seeding
• ✅ Ambisolver finding performance | trackfinding | ttbar with 200 pileup | default seeding
• ✅ AMVF gauss notime | trackfinding | ttbar with 200 pileup | default seeding
• ✅ AMVF grid time | trackfinding | ttbar with 200 pileup | default seeding

[andiwand]

I wonder if the nan is bound to an earlier FPE and we should rather fix the root cause. Something like that might help https://github.com/acts-project/acts/pull/3756/files#diff-c0c115f048fe0bf709beaf5d8268193cb06dcadcda1819c7722696393e4e2df2R49

I have this helper

acts/Core/include/Acts/Utilities/AngleHelpers.hpp

Lines 15 to 23 in 645a8d1

	/// Calculate the pseudorapidity from the polar angle theta.
	///
	/// @param theta is the polar angle in radian towards the z-axis.
	///
	/// @return the pseudorapidity towards the z-axis.
	template <typename Scalar>
	Scalar etaFromTheta(Scalar theta) {
	return -std::log(std::tan(0.5 * theta));
	}

and would like to make it FPE safe. In that case it could also fix the problem you see at its root.

[timadye]

I wonder if the nan is bound to an earlier FPE and we should rather fix the root cause.

I don't know if FPEs come from std::log(-ve) giving nan. In the Athena job, I am seeing ~1 FPE (WARNING FPE INVALID in [Execute] of [ActsToTrkConvertorAlg] on event ...) an event, but not for the event that throwed the exception.

#3756 sounds like an excellent idea. I will take a look in detail. Even so, I think we should be protected against nans in this code, as users can pass what they like.

[andiwand]

I think nan inputs we see as a user responsibility but let's also pull in @paulgessinger for this. I would hope that an FPE pops up if a nan is produced in some way

The merge-base changed after approval.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
68.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[acts-project-service]

✅ Athena integration test results

✅ All tests successful

status	job	report
🟢	report_pull_request
🟢	run_art_test: test_run4_acts_vertex_PU200
🟢	run_art_test: test_run4_ttbar_PU200
🟢	run_art_test: test_data18_13TeV_1000evt
🟢	run_art_test: test_ttbarPU40_reco

[paulgessinger]

Somehow this PR failed physmon on GitLab after having been merged. But is was green here, so it might be a fluke?

[timadye]

Somehow #3794's changes were included here and removed from that PR. No harm done.