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

ROOT 6.32 validation #45556

Closed
smuzaffar opened this issue Jul 25, 2024 · 59 comments
Closed

ROOT 6.32 validation #45556

smuzaffar opened this issue Jul 25, 2024 · 59 comments

Comments

@smuzaffar
Copy link
Contributor

@makortel , CMSSW_14_1_0_pre6 is in build phase, should we build CMSSW_14_1_0_pre6_ROOT632 to start the validation of root 6.32 for it possible integration in CMSSW 15.0 ?

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 25, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

A new Issue was created by @smuzaffar.

@Dr15Jones, @antoniovilela, @makortel, @mandrenguyen, @rappoccio, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@makortel
Copy link
Contributor

assign core, pdmv

@cmsbuild
Copy link
Contributor

New categories assigned: core,pdmv

@Dr15Jones,@AdrianoDee,@sunilUIET,@miquork,@makortel,@smuzaffar,@kskovpen you have been requested to review this Pull request/Issue and eventually sign? Thanks

@makortel
Copy link
Contributor

should we build CMSSW_14_1_0_pre6_ROOT632 to start the validation of root 6.32 for it possible integration in CMSSW 15.0 ?

Let's build it.

@smuzaffar
Copy link
Contributor Author

I have opened #45557

@smuzaffar
Copy link
Contributor Author

CMSSW_14_1_0_pre6_ROOT632 is available now (#45557)
Release validation ticket: https://its.cern.ch/jira/browse/PDMVRELVALS-249

@smuzaffar
Copy link
Contributor Author

CMSSW_14_1_0_pre6_ROOT632 Relval campaign: https://cms-pdmv-prod.web.cern.ch/valdb/campaigns/search/root632

@AdrianoDee
Copy link
Contributor

Some issues spotted by electrons. Consider that the samples share exactly the same GEN events (for noPU, for PU we re-run only from RECO).

@makortel
Copy link
Contributor

Thanks @AdrianoDee. Is this difference being investigated already?

@AdrianoDee
Copy link
Contributor

Let me tag @SanghyunKo and @cochando as EGM RECO conveners so that they can have a look at it.

@SanghyunKo
Copy link
Contributor

SanghyunKo commented Oct 24, 2024

Is the only difference between the two releases supposed to be the ROOT version (6.30/07 vs 6.32)?

@AdrianoDee
Copy link
Contributor

Yes

@AdrianoDee
Copy link
Contributor

You can have a look at the campaign description just to have an idea of all the samples produced. There are links to cmsDrivers, DAS.

@makortel
Copy link
Contributor

@SanghyunKo @cochando Have you been able to take a look?

@makortel
Copy link
Contributor

@AdrianoDee progress on the validation seems slow... E.g. for the failure in electrons, given the success in ECAL, reports from tracking and tracker (maybe photons?) would be interesting.

@AdrianoDee
Copy link
Contributor

AdrianoDee commented Nov 12, 2024

@makortel I'll ask tracking/tracker/photons validators to have a look. Unfortunately, validators can't go much faster given the number of validations we regularly have. And on top of that we had to prioritize the validation campaigns for the MC productions. I'll keep an eye on that to see how it progress (technically the deadline is two days from now).

@SanghyunKo
Copy link
Contributor

We looked into various plots for electrons, but still struggling to understand what happened here. The problematic variable is E/p, and the ECAL variable looks okay... which suggest the tracking issue. The puzzling thing is that the problematic region is 2.5 < |η| < 3, where we have no tracker (but only pixels). So it would be helpful if there is any independent report related to the pixel (seeding).

@makortel
Copy link
Contributor

I'll ask tracking/tracker/photons validators to have a look. Unfortunately, validators can't go much faster given the number of validations we regularly have. And on top of that we had to prioritize the validation campaigns for the MC productions. I'll keep an eye on that to see how it progress (technically the deadline is two days from now).

@AdrianoDee Any news?

@makortel
Copy link
Contributor

@SanghyunKo @cochando Has anyone looked at the electrons in data? (I didn't see a report in https://cms-pdmv-prod.web.cern.ch/valdb/campaigns/search/root632)

@srimanob
Copy link
Contributor

srimanob commented Nov 21, 2024

Hi @cms-sw/pdmv-l2 @SanghyunKo

Could you please clarify about samples you are using? I see difference results when I try to compare between STDs, and between KITFARM_SpecialRVs. Do I understand correctly that issue shows up only when you share the same GEN, not the same RAW.

Thanks.

With STDs: I don't see different in the EoP plot:
/RelValZEE_14/CMSSW_14_1_0_pre6-140X_mcRun3_2024_realistic_v15_STD_2024_PU-v1/DQMIO
/RelValZEE_14/CMSSW_14_1_0_pre6_ROOT632-140X_mcRun3_2024_realistic_v15_STD_RecoOnly_2024_PU-v1/DQMIO
Both share the same RAW: /RelValZEE_14/CMSSW_14_1_0_pre6-PU_140X_mcRun3_2024_realistic_v15_STD_2024_PU-v1/GEN-SIM-DIGI-RAW
DQM: https://tinyurl.com/2atp76fg
ZEE_14TeV_STD

With KITFARM_SpecialRVs (the ones that EGamma use from plot definition), I see the difference in the EoP plot as reported:
/RelValZEE_14/CMSSW_14_1_0_pre6-140X_mcRun3_2024_realistic_v15_2024_KITFARM_x86_SpecialRV-v1/DQMIO
/RelValZEE_14/CMSSW_14_1_0_pre6_ROOT632-140X_mcRun3_2024_realistic_v15_2024_KITFARM_ROOT632_SpecialRV-v1/DQMIO
Both share the same GEN: /RelValZEE_14/CMSSW_14_1_0_pre6-140X_mcRun3_2024_realistic_v15_2024_GENOnly_KITFARM_x86_SpecialRV-v1/GEN
DQM: https://tinyurl.com/2annk7cd
ZEE_14TeV_KITFARM

@srimanob
Copy link
Contributor

srimanob commented Nov 21, 2024

I think I can reproduce this strange behaviors.
Black: ROOT630
Blue: ROOT632, same RAWSIM, RECO only
Orange: ROOT632, New sample from scratch
Something is going strange somewhere in SIM or DIGI steps.
DQM: https://tinyurl.com/29gy99bl

Screenshot 2567-11-21 at 21 41 12

@srimanob
Copy link
Contributor

srimanob commented Nov 21, 2024

Tracker DIGI seems to show discrepancies:

NoPU:
Strip: https://tinyurl.com/2xzrjx39
PX: https://tinyurl.com/2chjhahg (barrel) https://tinyurl.com/28ykjsbl (forward)
it seems off in low ADC counts

@cms-sw/trk-dpg-l2 @henriettepetersen @sroychow
Could you please have look? Any modules that may affect heavily from ROOT?

@srimanob
Copy link
Contributor

srimanob commented Nov 22, 2024

Hi,
I found that when I fix the pDetIDAssociationInfo here which is may be used uninitialized, I get a converge result. It is not fully converge to the ROOT 630 but clearly improve. Result seems to be in good agreement when the fix is applied in both 630 and 632. See plot below.

Still no clue why ROOT plays role here, @dpiparo @makortel any clues? Or it is just memory effect which fix, I don't know.
Plot uses the same GEN-SIM, DQM in https://tinyurl.com/24jmmovz
Black = ROOT 630
Blue = ROOT630 + Fix
Orange = ROOT 632
Red = ROOT 632 + Fix

Screenshot 2567-11-22 at 08 41 00

@srimanob
Copy link
Contributor

srimanob commented Nov 22, 2024

By the way, from EGamma report, I get a confusion a bit. Red is 630, Blue is 632. It seems in opposite on what I get.

Screenshot 2567-11-22 at 08 58 49

@makortel
Copy link
Contributor

Thanks @srimanob! (reminds me of past technical validation rounds, e.g. #36339)

I wonder though if these plots have shown any "false positives" before?

@srimanob
Copy link
Contributor

srimanob commented Nov 22, 2024

Hi @makortel
Seem not, this round is 50% off.
Here is the link of ROOT 624 validation:
Screenshot 2567-11-23 at 00 51 54

@AdrianoDee
Copy link
Contributor

Thanks Phat! I'm sending some 100k wfs (for the flagship samples TTbar, Zee, ...) just to close the loop. As a note to us (PdmV) for the next rounds we should produce very high stats samples, at least for some core processes (again TTbar etc.). For this round we just doubled the usual stats.

@beaudett
Copy link
Contributor

Hi @makortel
There has been differences for this plot in the previous validations, but limited to one sample. What triggered the alarm here was the fact we could see deviations at the same place in different samples

@beaudett
Copy link
Contributor

Hi,
I have updated the report
If you agree, I propose to change the status to ok.

@smuzaffar
Copy link
Contributor Author

@makortel , validation reports seems good now ( though many are missing). should we move to root 6.32 for 15.0.X now?

@srimanob
Copy link
Contributor

@cms-sw/pdmv-l2
Do you still waiting validation reports? Thx.

@AdrianoDee
Copy link
Contributor

Yes, there are still some I'd like you to see there. Do you have a deadline on your side cor integrating this?

@makortel
Copy link
Contributor

makortel commented Dec 2, 2024

validation reports seems good now ( though many are missing). should we move to root 6.32 for 15.0.X now?

I wonder if we want to have the microarchitecture change (x86-64-v2 to v3) validated in the same prerelease where we deploy ROOT 6.32? (while theoretically the ROOT update is validated here, I wouldn't be surprised if we'd see similar statistical fluctuations in the pre-release where the ROOT update is deployed)

I was originally thinking we'd do the microarchitecture change in pre1, but I'm not sure if the discussion (even on the "big picture) in dmwm/WMCore#12168 would converge "soon" (ok, I haven't fully digested the comments made since last Wednesday).

@smuzaffar
Copy link
Contributor Author

smuzaffar commented Dec 2, 2024

I wonder if we want to have the microarchitecture change (x86-64-v2 to v3) validated in the same prerelease where we deploy ROOT 6.32? (while theoretically the ROOT update is validated here, I wouldn't be surprised if we'd see similar statistical fluctuations in the pre-release where the ROOT update is deployed)

good point @makortel . I agree, both v3 validation and root 6.32 should go in different pre-releases.

About, v3 validation, as validation jobs only run at FNAL or CERN (both has only >=v3 resources) , so we can enable v3 in 15.0.0.pre1.

dmwm/WMCore#12168 mostly need changes for CRAB/Analysis jobs, so we have time to converge.

@makortel
Copy link
Contributor

makortel commented Dec 2, 2024

Yes, there are still some I'd like you to see there. Do you have a deadline on your side cor integrating this?

@AdrianoDee If we'd want to deploy ROOT 6.32 for pre1 (but see the discussion on microarchitecture), we should make do the update (i.e. merge cms-sw/cmsdist#9541) about now (in order to get it tested in all IB flavors, and have some time to react in case of problems, before pre1 will be built next week).

We should deploy 6.32 in pre2 at the latest (second-to-last prerelease), so counting backwards

  • Jan 14 pre2 is built
  • Jan 7 need to deploy ROOT 6.32 in master
  • However, CERN has just opened on Jan 6, and we can probably reasonably expect only little progress on the validation during the 2 weeks of CERN closure. I think we should also avoid the deployment right before the CERN closure, so in a way a preferable deployment time would be around Dec 16 at the latest (and after pre1 has been built).

So there is about a week to two weeks to conclude (or to Jan 6 if really really needed).

@AdrianoDee
Copy link
Contributor

AdrianoDee commented Dec 2, 2024

Thanks @makortel, given the current status of the validations, I think we can close it by this week. I'll ping the validators that have not reported yet and we can check the status at next week ORP/Core meeting.

@AdrianoDee
Copy link
Contributor

(I edited the comment above. I think we should be able to close the validation this week. Then for ROOT integrated in pre1 depends a bit on when we'll end up cutting it.)

@AdrianoDee
Copy link
Contributor

Just to say that, while we still don't have all the reports, for me we have the quorum to say the validation was successful.

@makortel
Copy link
Contributor

Thanks @AdrianoDee! @smuzaffar Should we deploy ROOT 6.32 in default IB early next week (after the pre1 has been built)?

@smuzaffar
Copy link
Contributor Author

Thanks @AdrianoDee! @smuzaffar Should we deploy ROOT 6.32 in default IB early next week (after the pre1 has been built)?

yes, sounds good.

@smuzaffar
Copy link
Contributor Author

ROOT 6.32 is now integrated in 15.0.X IBs. I also have replaced ROOT632 IBs with ROOT634 Ibs

@AdrianoDee
Copy link
Contributor

+pdmv
Validation successful

@makortel
Copy link
Contributor

+core

@makortel
Copy link
Contributor

@cmsbuild, please close

@cmsbuild
Copy link
Contributor

This issue is fully signed and ready to be closed.

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

No branches or pull requests

7 participants