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

[O2B-1365] Disable GAQ for not fully covered runs #1784

Open
wants to merge 42 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
fc4f46f
add run bounds to gaq eff. periods
xsalonx Oct 12, 2024
76848cb
WIP
xsalonx Oct 12, 2024
e95b06d
empty ranges work
xsalonx Oct 12, 2024
782284c
gaq summary WIP
xsalonx Oct 12, 2024
4978810
WIP
xsalonx Oct 12, 2024
113c6aa
work
xsalonx Oct 14, 2024
3b39854
simplify
xsalonx Oct 14, 2024
8ef35d6
fix
xsalonx Oct 14, 2024
ec92f8d
gaq summary disabled when missing coverage
xsalonx Oct 14, 2024
ee7e2d2
Merge branch 'main' into xsalonx/gaq/O2B-1365/disable-gaq-if-detector…
xsalonx Nov 19, 2024
ff09301
add comments
xsalonx Nov 19, 2024
b9b163c
add comments
xsalonx Nov 19, 2024
ae4d996
remove tf timestamps form query
xsalonx Nov 19, 2024
baa9bda
fix joins
xsalonx Nov 19, 2024
5880df0
a
xsalonx Nov 19, 2024
27e3a7a
add seeders
xsalonx Nov 20, 2024
7a2d17d
fix merging summaries
xsalonx Nov 20, 2024
adb74ce
cleanup
xsalonx Nov 20, 2024
42c479d
docs
xsalonx Nov 20, 2024
d37c942
fix tests
xsalonx Nov 20, 2024
5ae8225
add colors
xsalonx Nov 20, 2024
ad3e415
fix seeder
xsalonx Nov 20, 2024
87c7038
fix test
xsalonx Nov 20, 2024
eb3dab5
fix api test
xsalonx Nov 20, 2024
727980d
fix
xsalonx Nov 20, 2024
28b5931
fix test
xsalonx Nov 20, 2024
42513a1
ch display, fix tests
xsalonx Nov 20, 2024
1dff087
fix
xsalonx Nov 20, 2024
fb67a69
fix test
xsalonx Nov 21, 2024
3b9cbd0
test fix
xsalonx Nov 21, 2024
5f2f0ed
fix test
xsalonx Nov 21, 2024
f669198
fix test
xsalonx Nov 21, 2024
6b3aee3
docs
xsalonx Nov 21, 2024
dc93796
rename
xsalonx Nov 27, 2024
b13e82e
test
xsalonx Nov 27, 2024
d862b16
Add QcSummaryUnit wrapping class
xsalonx Nov 27, 2024
bcd147e
megre main
xsalonx Nov 27, 2024
ad1ab15
fix test
xsalonx Nov 27, 2024
1f50297
fix test
xsalonx Nov 28, 2024
1632aa9
fix
xsalonx Nov 28, 2024
570f25c
fix test
xsalonx Nov 28, 2024
40bb12d
Merge branch 'main' into xsalonx/gaq/O2B-1365/disable-gaq-if-detector…
xsalonx Dec 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 69 additions & 34 deletions lib/database/repositories/QcFlagRepository.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,26 @@ const GAQ_PERIODS_VIEW = `
ROWS BETWEEN CURRENT ROW AND 1 FOLLOWING
) AS \`to\`
FROM (
-- Two selects for runs' timestamps (in case QC flag's eff. period doesn't start at run's start or end at run's end )
Copy link
Collaborator

Choose a reason for hiding this comment

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

This raw SQL querries start to be very complex. Could you try to create a view that would simplify this?
First, you should already use time_start and time_end that already coalesce time_trg_start and time_trg_end
Also, coalesce null timestamp to 0 works but is a bit wrong in my opinion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

applied in xsalonx/gaq/O2B-1365/disable-gaq-if-detector-is-missing---with-views

(
SELECT gaqd.data_pass_id,
gaqd.run_number,
COALESCE(UNIX_TIMESTAMP(COALESCE(time_trg_start, time_o2_start)), 0) AS timestamp
FROM global_aggregated_quality_detectors AS gaqd
INNER JOIN runs as r
ON gaqd.run_number = r.run_number
)
UNION
(
SELECT gaqd.data_pass_id,
gaqd.run_number,
UNIX_TIMESTAMP(COALESCE(time_trg_end, time_o2_end, NOW())) AS timestamp
FROM global_aggregated_quality_detectors AS gaqd
INNER JOIN runs as r
ON gaqd.run_number = r.run_number
)
UNION
-- Two selectes for timestamps of QC flags' effective periods
(
SELECT gaqd.data_pass_id,
gaqd.run_number,
Expand Down Expand Up @@ -106,22 +126,28 @@ class QcFlagRepository extends Repository {
gaq_periods.run_number AS runNumber,
IF(gaq_periods.\`from\` = 0, null, gaq_periods.\`from\` * 1000) AS \`from\`,
IF(gaq_periods.\`to\` = UNIX_TIMESTAMP(NOW()), null, gaq_periods.\`to\` * 1000) AS \`to\`,
group_concat(qcf.id) AS contributingFlagIds
group_concat(qcfep.flag_id) AS contributingFlagIds

FROM quality_control_flags AS qcf
INNER JOIN quality_control_flag_effective_periods AS qcfep
ON qcf.id = qcfep.flag_id
INNER JOIN data_pass_quality_control_flag AS dpqcf ON dpqcf.quality_control_flag_id = qcf.id
INNER JOIN (${GAQ_PERIODS_VIEW}) AS gaq_periods ON gaq_periods.data_pass_id = dpqcf.data_pass_id
FROM (${GAQ_PERIODS_VIEW}) AS gaq_periods
INNER JOIN global_aggregated_quality_detectors AS gaqd
ON gaqd.data_pass_id = gaq_periods.data_pass_id
AND gaqd.run_number = gaq_periods.run_number
AND gaqd.detector_id = qcf.detector_id
AND gaq_periods.run_number = qcf.run_number
AND (qcfep.\`from\` IS NULL OR UNIX_TIMESTAMP(qcfep.\`from\`) <= gaq_periods.\`from\`)
AND (qcfep.\`to\` IS NULL OR gaq_periods.\`to\` <= UNIX_TIMESTAMP(qcfep.\`to\`))
AND gaqd.run_number = gaq_periods.run_number
LEFT JOIN (
data_pass_quality_control_flag AS dpqcf
INNER JOIN quality_control_flags AS qcf
ON dpqcf.quality_control_flag_id = qcf.id
INNER JOIN quality_control_flag_effective_periods AS qcfep
ON qcf.id = qcfep.flag_id
)
ON gaq_periods.data_pass_id = dpqcf.data_pass_id
AND qcf.run_number = gaq_periods.run_number
AND gaqd.detector_id = qcf.detector_id
AND gaq_periods.run_number = qcf.run_number
AND (qcfep.\`from\` IS NULL OR UNIX_TIMESTAMP(qcfep.\`from\`) <= gaq_periods.\`from\`)
AND (qcfep.\`to\` IS NULL OR gaq_periods.\`to\` <= UNIX_TIMESTAMP(qcfep.\`to\`))

WHERE gaq_periods.data_pass_id = ${dataPassId}
WHERE IF(gaq_periods.\`to\` = UNIX_TIMESTAMP(NOW()), null, gaq_periods.\`to\` * 1000) IS NOT NULL
AND gaq_periods.data_pass_id = ${dataPassId}
${runNumber ? `AND gaq_periods.run_number = ${runNumber}` : ''}

GROUP BY gaq_periods.run_number,
Expand All @@ -142,7 +168,7 @@ class QcFlagRepository extends Repository {
runNumber,
from,
to,
contributingFlagIds: contributingFlagIds.split(',').map((id) => parseInt(id, 10)),
contributingFlagIds: contributingFlagIds ? contributingFlagIds.split(',').map((id) => parseInt(id, 10)) : [],
}));
}

Expand All @@ -162,35 +188,44 @@ class QcFlagRepository extends Repository {
gaq_periods.run_number AS runNumber,
IF(gaq_periods.\`from\` = 0, null, gaq_periods.\`from\`) AS \`from\`,
IF(gaq_periods.\`to\` = UNIX_TIMESTAMP(NOW()), null, gaq_periods.\`to\`) AS \`to\`,
SUM(IF(qcft.monte_carlo_reproducible AND :mcReproducibleAsNotBad, false, qcft.bad)) >= 1 AS bad,
IF(COUNT( DISTINCT gaqd.detector_id ) > COUNT( DISTINCT qcfep.flag_id ),
null,
SUM(IF(qcft.monte_carlo_reproducible AND :mcReproducibleAsNotBad, false, qcft.bad)) >= 1
) AS bad,
SUM(qcft.bad) = SUM(qcft.monte_carlo_reproducible) AND SUM(qcft.monte_carlo_reproducible) AS mcReproducible,
GROUP_CONCAT( DISTINCT qcfv.flag_id ) AS verifiedFlagsList,
GROUP_CONCAT( DISTINCT qcf.id ) AS flagsList
GROUP_CONCAT( DISTINCT qcfep.flag_id ) AS flagsList

FROM quality_control_flags AS qcf
INNER JOIN quality_control_flag_types AS qcft
ON qcft.id = qcf.flag_type_id
LEFT JOIN quality_control_flag_verifications AS qcfv
ON qcfv.flag_id = qcf.id
INNER JOIN quality_control_flag_effective_periods AS qcfep
ON qcf.id = qcfep.flag_id
INNER JOIN data_pass_quality_control_flag AS dpqcf
ON dpqcf.quality_control_flag_id = qcf.id
INNER JOIN (${GAQ_PERIODS_VIEW}) AS gaq_periods
ON gaq_periods.data_pass_id = dpqcf.data_pass_id
FROM (${GAQ_PERIODS_VIEW}) AS gaq_periods
INNER JOIN global_aggregated_quality_detectors AS gaqd
ON gaqd.data_pass_id = gaq_periods.data_pass_id
AND gaqd.run_number = gaq_periods.run_number
AND gaqd.detector_id = qcf.detector_id
AND gaq_periods.run_number = qcf.run_number
AND (qcfep.\`from\` IS NULL OR UNIX_TIMESTAMP(qcfep.\`from\`) <= gaq_periods.\`from\`)
AND (qcfep.\`to\` IS NULL OR gaq_periods.\`to\` <= UNIX_TIMESTAMP(qcfep.\`to\`))
AND gaqd.run_number = gaq_periods.run_number

LEFT JOIN (
data_pass_quality_control_flag AS dpqcf
INNER JOIN quality_control_flags AS qcf
ON dpqcf.quality_control_flag_id = qcf.id
INNER JOIN quality_control_flag_types AS qcft
ON qcft.id = qcf.flag_type_id
INNER JOIN quality_control_flag_effective_periods AS qcfep
ON qcf.id = qcfep.flag_id
LEFT JOIN quality_control_flag_verifications AS qcfv
ON qcfv.flag_id = qcf.id
)
ON gaq_periods.data_pass_id = dpqcf.data_pass_id
AND qcf.run_number = gaq_periods.run_number
AND gaqd.detector_id = qcf.detector_id
AND gaq_periods.run_number = qcf.run_number
AND (qcfep.\`from\` IS NULL OR UNIX_TIMESTAMP(qcfep.\`from\`) <= gaq_periods.\`from\`)
AND (qcfep.\`to\` IS NULL OR gaq_periods.\`to\` <= UNIX_TIMESTAMP(qcfep.\`to\`))

WHERE gaq_periods.\`to\` IS NOT null

GROUP BY
gaq_periods.data_pass_id,
gaq_periods.run_number,
gaq_periods.\`from\`,
gaq_periods.\`to\`
IF(gaq_periods.\`from\` = 0, null, gaq_periods.\`from\`),
IF(gaq_periods.\`to\` = UNIX_TIMESTAMP(NOW()), null, gaq_periods.\`to\`)
`;

const query = `
Expand Down Expand Up @@ -256,7 +291,7 @@ class QcFlagRepository extends Repository {
bad,
effectiveRunCoverage: parseFloat(effectiveRunCoverage) || null,
mcReproducible: Boolean(mcReproducible),
flagsIds: [...new Set(flagsList.split(','))],
flagsIds: flagsList ? [...new Set(flagsList.split(','))] : [],
verifiedFlagsIds: verifiedFlagsList ? [...new Set(verifiedFlagsList.split(','))] : [],
}));
}
Expand Down
12 changes: 8 additions & 4 deletions lib/database/seeders/20240213120811-qc-flags-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ module.exports = {
bad: true,
created_by_id: 1,
monte_carlo_reproducible: false,
color: '#d62631',
},
{
id: 3,
Expand All @@ -32,22 +33,23 @@ module.exports = {
bad: false,
created_by_id: 1,
monte_carlo_reproducible: false,
color: '#4caf50',
},
{
id: 5,
name: 'Limited Acceptance MC Reproducible',
method: 'LimitedAcceptanceMCReproducible',
monte_carlo_reproducible: true,
bad: true,
color: '#FFFF00',
color: '#bb9d30',
created_by_id: 1,
},
{
id: 11,
name: 'Limited acceptance',
method: 'LimitedAcceptance',
name: 'Limited Acceptance MC Not Reproducible',
method: 'LimitedAcceptanceMCNotReproducible',
bad: true,
color: '#FFFF00',
color: '#72512c',
created_by_id: 1,
monte_carlo_reproducible: false,
},
Expand All @@ -58,6 +60,7 @@ module.exports = {
bad: true,
created_by_id: 1,
monte_carlo_reproducible: false,
color: '#d62631',
},
{
id: 13,
Expand All @@ -66,6 +69,7 @@ module.exports = {
bad: true,
created_by_id: 1,
monte_carlo_reproducible: false,
color: '#d62631',
},
{
id: 20,
Expand Down
102 changes: 102 additions & 0 deletions lib/database/seeders/20240404100811-qc-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,65 @@ module.exports = {
updated_at: '2024-02-13 11:58:20',
},

/** Flags for runNumber: 56, LHC22a_apass1, FT0 */
{
id: 10,
from: '2019-08-08 20:00:00',
to: '2019-08-08 21:00:00',
comment: 'Some qc comment 10',

// Associations
created_by_id: 2,
flag_type_id: 3, // Good
run_number: 56,
detector_id: 7, // FT0
created_at: '2024-02-13 11:58:20',
updated_at: '2024-02-13 11:58:20',
},
{
id: 13,
from: '2019-08-08 20:00:00',
to: '2019-08-08 20:30:00',
comment: 'Some qc comment 10',

// Associations
created_by_id: 2,
flag_type_id: 5, // Good
run_number: 56,
detector_id: 7, // FT0
created_at: '2024-02-13 11:58:20',
updated_at: '2024-02-13 11:58:20',
},
// For ITS
{
id: 11,
from: '2019-08-08 20:00:00',
to: '2019-08-08 21:00:00',
comment: 'Some qc comment 11',

// Associations
created_by_id: 2,
flag_type_id: 3, // Good
run_number: 56,
detector_id: 4, // ITS
created_at: '2024-02-13 11:58:20',
updated_at: '2024-02-13 11:58:20',
},
{
id: 12,
from: '2019-08-08 20:30:00',
to: '2019-08-08 21:00:00',
comment: 'Some qc comment 12',

// Associations
created_by_id: 2,
flag_type_id: 5, // Lim. Acc. MC Reproducible
run_number: 56,
detector_id: 4, // ITS
created_at: '2024-02-13 11:58:20',
updated_at: '2024-02-13 11:58:20',
},

/** Synchronous */

// Run : 56, FT0
Expand Down Expand Up @@ -214,6 +273,33 @@ module.exports = {
to: '2019-08-09 14:00:00',
},

{
id: 10,
flag_id: 10,
from: '2019-08-08 20:30:00',
to: '2019-08-08 21:00:00',
},
{
id: 13,
flag_id: 13,
from: '2019-08-08 20:00:00',
to: '2019-08-08 20:30:00',
},

{
id: 11,
flag_id: 11,
from: '2019-08-08 20:00:00',
to: '2019-08-08 20:30:00',
},

{
id: 12,
flag_id: 12,
from: '2019-08-08 20:30:00',
to: '2019-08-08 21:00:00',
},

/** Synchronous */
// Run : 56, FT0
{
Expand Down Expand Up @@ -260,6 +346,22 @@ module.exports = {
data_pass_id: 2, // LHC22b_apass2
quality_control_flag_id: 4,
},
{
data_pass_id: 3, // LHC22a_apass1
quality_control_flag_id: 10,
},
{
data_pass_id: 3, // LHC22a_apass1
quality_control_flag_id: 13,
},
{
data_pass_id: 3, // LHC22a_apass1
quality_control_flag_id: 11,
},
{
data_pass_id: 3, // LHC22a_apass1
quality_control_flag_id: 12,
},
], { transaction }),

queryInterface.bulkInsert('simulation_pass_quality_control_flag', [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
* @return {Component} display of aggregated quality
*/
const formatGeneralQuality = (contributingFlags, gaqDetectors) => {
const missingFlags = contributingFlags.length !== gaqDetectors.length;

Check warning on line 31 in lib/public/views/QcFlags/ActiveColumns/gaqFlagsActiveColumns.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/QcFlags/ActiveColumns/gaqFlagsActiveColumns.js#L31

Added line #L31 was not covered by tests
const flagTypes = contributingFlags.map(({ flagType }) => flagType);
const allGood = flagTypes.every(({ bad }) => !bad);
const someBadNotReproducible = flagTypes.some(({ bad, mcReproducible }) => bad && !mcReproducible);
Expand All @@ -44,7 +45,9 @@
: null;

let qualityDisplay = null;
if (allGood) {
if (missingFlags) {
qualityDisplay = badge('ND', { color: QcSummaryColors.INCALCULABLE_COVERAGE });
} else if (allGood) {

Check warning on line 50 in lib/public/views/QcFlags/ActiveColumns/gaqFlagsActiveColumns.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/QcFlags/ActiveColumns/gaqFlagsActiveColumns.js#L48-L50

Added lines #L48 - L50 were not covered by tests
qualityDisplay = badge('good', { color: QcSummaryColors.ALL_GOOD });
} else if (someBadNotReproducible) {
qualityDisplay = badge('bad', { color: QcSummaryColors.ALL_BAD });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@
visible: true,
format: (_, { runNumber }) => {
const runGaqSummary = gaqSummary[runNumber];
const gaqDisplay = runGaqSummary
const gaqDisplay = runGaqSummary?.qualityNotDefinedEffectiveRunCoverage === 0

Check warning on line 170 in lib/public/views/Runs/RunPerDataPass/RunsPerDataPassOverviewPage.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Runs/RunPerDataPass/RunsPerDataPassOverviewPage.js#L170

Added line #L170 was not covered by tests
? getQcSummaryDisplay(runGaqSummary, { mcReproducibleAsNotBad })
: h('button.btn.btn-primary.w-100', 'GAQ');

Expand Down
Loading