Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix a bunch of failing unit tests after D88 and D92 went missing #45764

Merged
merged 3 commits into from
Aug 24, 2024

Conversation

mmusich
Copy link
Contributor

@mmusich mmusich commented Aug 21, 2024

PR description:

Title says it all, in partial response to #45370 (comment).

PR validation:

Most of the tests now run correctly: scram b runtests of the changed packages.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

N/A
Cc: @srimanob

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 21, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mmusich for master.

It involves the following packages:

  • Alignment/OfflineValidation (alca)
  • Alignment/TrackerAlignment (alca)
  • CondTools/SiPhase2Tracker (upgrade, alca)
  • Configuration/Geometry (upgrade, geometry)
  • Geometry/TrackerGeometryBuilder (geometry)
  • RecoTracker/MkFit (reconstruction)
  • SLHCUpgradeSimulations/Geometry (upgrade, geometry)
  • SimTracker/TrackerMaterialAnalysis (simulation)
  • Validation/Geometry (dqm, geometry)

@Dr15Jones, @antoniovagnerini, @bsunanda, @civanch, @cmsbuild, @consuegs, @jfernan2, @makortel, @mandrenguyen, @mdhildreth, @nothingface0, @perrotta, @rvenditti, @saumyaphor4252, @srimanob, @subirsarkar, @syuvivida, @tjavaid can you please review it and eventually sign? Thanks.
@GiacomoSguazzoni, @JanFSchulte, @Martin-Grunewald, @VinInn, @VourMa, @adewit, @apsallid, @bsunanda, @dgulhan, @fabiocos, @felicepantaleo, @gpetruc, @makortel, @martinamalberti, @missirol, @mmusich, @mtosi, @rbartek, @rishabhCMS, @rovere, @rsreds, @slomeo, @threus, @tlampen, @tocheng, @vargasa, @venturia, @youyingli, @yuanchao this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@mmusich
Copy link
Contributor Author

mmusich commented Aug 21, 2024

@cmsbuild, please test

Copy link
Contributor Author

@mmusich mmusich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cms-sw/trk-dpg-l2 this leads me to believe there is something wrong in the TrackerTopology implementation of geometry T35 (in D110). Perhaps it's linked to the existing issue #43097.

Please take a look.

@@ -46,15 +46,15 @@
###################################################################
# Ideal geometry producer and standard includes
###################################################################
process.load('Configuration.Geometry.GeometryExtended2026D88Reco_cff')
process.load('Configuration.Geometry.GeometryExtended2026DefaultReco_cff')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately when running runtests_test_CreateRandomMisalignment this leads to:

----- Begin Fatal Exception 21-Aug-2024 12:54:53 CEST-----------------------
An exception of category 'BadHierarchy' occurred while
   [0] Calling EventProcessor::runToCompletion (which does almost everything after beginJob and before endJob)
Exception Message:
[AlignableTrackerBuilder] Pixel GeomDet (subdetector 1) is not a GeomDetUnit.
----- End Fatal Exception -------------------------------------------------

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not yet solved, but circumvented for the time being by using a different geometry 2026D107 instead.

@cmsbuild
Copy link
Contributor

@civanch
Copy link
Contributor

civanch commented Aug 23, 2024

+1

@srimanob
Copy link
Contributor

+Upgrade

@jfernan2
Copy link
Contributor

+1

@mandrenguyen
Copy link
Contributor

@cms-sw/dqm-l2 please have a look

@tjavaid
Copy link

tjavaid commented Aug 24, 2024

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (test failures were overridden). This pull request will now be reviewed by the release team before it's merged. @mandrenguyen, @antoniovilela, @rappoccio, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2)

@mandrenguyen
Copy link
Contributor

+1
merging #45775 and #45764 together

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment