Wrong formula for percentual difference in profiling

[felicepantaleo]

It is calculated as (PR-baseline)/PR while it should be calculated as (PR-baseline)/baseline. It is wrong in all the columns cpu time percent diff, allocated memory percent diff, deallocated memory percent diff

From a PR: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d4760a/42504/profiling/29834.21/diff-step3_cpu.resources.json.html
I see:

[cmsbuild]

cms-bot internal usage

[cmsbuild]

A new Issue was created by @felicepantaleo.

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

cms-bot commands are listed here

[smuzaffar]

assign core

[cmsbuild]

New categories assigned: core

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

[smuzaffar]

@gartung , can you please look in to this?

[gartung]

@smuzaffar can profiling be run on the baseline release. Part of the problem is that the "baseline" in this instance is the profiling for the last IB.

[smuzaffar]

sorry @gartung , I did not get your point ? PR profiling job should use the same IB for which the the PR tests were run.

[gartung]

I need to set the number of events to 100 and I wanted to avoid running that many for every IB.

[gartung]

Step 1 and step 2 are run for each job so technically the input for step3 RECO is different. On average the results should be the same but not with the limited stats of 10 events.

[smuzaffar]

ah ok, I thought we were doign it already. Aren't we downloading baseline results during the PR profiling job here https://github.com/cms-sw/cms-bot/blob/master/pr_testing/run-pr-profiling.sh#L50-L54 and here https://github.com/cms-sw/cms-bot/blob/master/pr_testing/run-pr-profiling.sh#L98 ?

[gartung]

No, it is pulling the data from the IB.

[smuzaffar]

sorry, can you please explain with an example e.g for PR1 if we have used IBa then IBa is baseline and profiling comparison should use the profiling data from IBa ... right? Are you saying that we use IBa for PR tests but downloaded profiling data for IBb ?

[smuzaffar]

shouldn't we just change https://github.com/cms-sw/cms-bot/blob/master/comparisons/resources-diff.py#L18-L19 to following?
I think data[metric] is for IB and dest[metric] is for Pr ... right ?

        if not data[metric] == 0.0:
            pdmetric = 100 * dmetric / data[metric]

[gartung]

Yes. I made that change and will submit a PR.

[gartung]

cms-sw/cms-bot#2365
It's failing black checks for code not touched by this PR.

[smuzaffar]

cms-sw/cms-bot#2365 is merged now. @gartung , please open a separate issue for #46578 (comment)

[makortel]

+core

[makortel]

@cmsbuild, please close

[cmsbuild]

This issue is fully signed and ready to be closed.