[PWGJE] Add PCH for JET tasks

[singiamtel]

Heavily speeds up compilation

CC @ktf

[github-actions]

O2 linter results: ❌ 1 errors, ⚠️ 0 warnings, 🔕 0 disabled

[fjonasALICE]

what is the time difference in compilation that you observe? should we consider this for other JE tasks too? also, are there some other headers that do not frequently change in O2Physics that we should consider precompiling?

[ktf]

@singiamtel very nice. Could you share numbers on the improvement? How does it handle changes in the framework headers? Will it actually notice and recompile? That's where the previous attempt I had did not converge.

[aalkin]

This is great, together with my changes at the framework level this will reduce the compilation times very significantly. However, I suggest restructuring this to make it more transparent for the end users, we do not want to rewrite every CMakeLists.txt in O2Physics. We can introduce a convention on the "central" target for each PWG, that collects all the universal headers and then update o2physics_add_dpl_workflow to automatically use it.

[ktf]

@vkucera on a separate note, could we please make sure the O2 linter does not tell people to change QA to Qa ?

[ktf]

@aalkin, as discussed offline and as I think we agree now, I think this approach (coupled with #14668) is actually better. We do need to have the common precompiled headers defined explicitly and to have the users select the appropriate one, like they do for dependencies.

[vkucera]

@vkucera on a separate note, could we please make sure the O2 linter does not tell people to change QA to Qa ?

Why?

[ktf]

Because the abbreviation of Quality Assurance is QA, not Qa.

[vkucera]

Because the abbreviation of Quality Assurance is QA, not Qa.

Yet we have McCollisions and McParticles, not MCCollisions and MCParticles, as names of tables and we have alice3-pid-rich-qa and hf-task-mc-validation, not a-l-i-c-e-3-p-i-d-r-i-c-h-q-a and h-f-task-m-c-validation, as names of workflows.

Capitalising only the first letter of each word-like segment is the only way to keep the CamelCase convention for structs, files, and types consistent with the kebab-case convention for workflows and devices.

[ktf]

I am not sure I understand. Can't you simply add the exceptions? There is hopefully a finite number of them. If the tools are wrong, we should fix the tools, or at least remove the check.

[vkucera]

I am not sure I understand. Can't you simply add the exceptions? There is hopefully a finite number of them. If the tools are wrong, we should fix the tools, or at least remove the check.

The error message is correct. O2 linter is checking name consistency in the full struct-device-workflow-file structure within the existing O2 conventions and algorithms.

• The text "Task QA" can be converted into lowerCamelCase either as taskQA or taskQa. Both are valid and both comply with the O2 naming conventions. (See the explicit examples numberOfDnsConnections, daqRate.)
• However, only taskQa can produce a correct kebab-case string task-qa when processed by type_to_task_name to generate the device name from a struct name. The taskQA string produces task-q-a.
• Moreover, only taskQa can be produced by the inverse conversion from the correct kebab-case task-qa. To get taskQA, one has to provide task-q-a.

Adding exceptions would defeat the purpose of testing consistency, break the compatibility with type_to_task_name, and allow accepting pathological cases like TPCTOFPIDQAMCTask.

[ktf]

@vkucera I am not sure why you say that the name of the file should be related to the name of the task inside. You can declare multiple tasks in the same workflow. That said, my statement still holds. Tools should not enforce something which is against common sense because something else is broken. We should either fix the broken part or leave the common sense alone and disable the check. In that regard, a better behaviour for type_to_task_name is to be found in AliceO2Group/AliceO2#15006. Can we have the linter stop complaining about QA and QC, please?

[vkucera]

@vkucera I am not sure why you say that the name of the file should be related to the name of the task inside. You can declare multiple tasks in the same workflow.

Because it's common sense to name files after their content. The struct-device-workflow-file name correspondence makes it trivial to deduce one name from another. E.g. if I want to know where a device is implemented, I get the file name by converting it to camelCase. Multiple tasks are not a problem. This was discussed one year ago and approved as error severity.

That said, my statement still holds. Tools should not enforce something which is against common sense because something else is broken. We should either fix the broken part or leave the common sense alone and disable the check.

The statement holds only if you ignore the role of capitalisation as a word divider in camelCase.

In that regard, a better behaviour for type_to_task_name is to be found in AliceO2Group/AliceO2#15006.

As I said, this introduces ambiguity and gives green light to unreadable names.

Can we have the linter stop complaining about QA and QC, please?

Of course, if you change the device name generation algorithm, I will have to adjust O2 linter accordingly. But this should have been discussed in a meeting with the analysis coordinator.

[ktf]

Because it's common sense to name files after their content. The struct-device-workflow-file name correspondence makes it trivial to deduce one name from another.

So does removing - and doing a case insensitive search.

E.g. if I want to know where a device is implemented, I get the file name by converting it to camelCase. Multiple tasks are not a problem.

I do git grep or fuzzy finding in my editor, even without needing the name of the file. Please do not call "common sense" the limitations of your tools or their configuration. You can get to it in many ways. I also question the actual usefulness of the correspondence, given we do support custom TaskNames and multiple tasks per file.

This was discussed one year ago and approved as error severity.

Yes, and in the same meeting I said that even one "false positive" to me it's enough to consider the check wrong and dangerous. Someone could have fixed the linter report by renaming the task to -q-a (which is less of a burden compared to changing the filename) and Hyperloop would have broken as well.

That said, my statement still holds. Tools should not enforce something which is against common sense because something else is broken. We should either fix the broken part or leave the common sense alone and disable the check.

The statement holds only if you ignore the role of capitalisation as a word divider in camelCase.

Yes, and as this PR shows, I am not the only one. Moreover, I question the word divider idea in the first place, because that would mean that hello-world and helloworld would have sit in two files differing only by capitalisation (which is a mess, and not only for macOS).

In that regard, a better behaviour for type_to_task_name is to be found in AliceO2Group/AliceO2#15006.

As I said, this introduces ambiguity and gives green light to unreadable names.

There is no ambiguity if you do the substitutions in reverse size order of the exceptions, as per comment in the code. Concerning unreadable names, TPCPIDQA.cxx is as unreadable as TpcPidQa.cxx.

Can we have the linter stop complaining about QA and QC, please?

Of course, if you change the device name generation algorithm, I will have to adjust O2 linter accordingly. But this should have been discussed in a meeting with the analysis coordinator.

Now that I know the implications, it will be in the meeting tomorrow, of course.

[singiamtel]

Superseded by #14690