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

[Anchor] Add some more help messages #1511

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

benedikt-voelkel
Copy link
Contributor

  • help messages
  • set ALIEN_JDL_ADDTIMESERIESINMC to a value at the beginning

* help messages
* set ALIEN_JDL_ADDTIMESERIESINMC to a value at the beginning
Copy link

REQUEST FOR PRODUCTION RELEASES:
To request your PR to be included in production software, please add the corresponding labels called "async-" to your PR. Add the labels directly (if you have the permissions) or add a comment of the form (note that labels are separated by a ",")

+async-label <label1>, <label2>, !<label3> ...

This will add <label1> and <label2> and removes <label3>.

The following labels are available
async-2022-pp-apass4
async-2023-pbpb-apass
async-2023-pp-apass1
async-data
async-mc
async-2022-pp-apass6

@catalinristea
Copy link
Collaborator

catalinristea commented Feb 29, 2024

Ciao @benedikt-voelkel, @chiarazampolli

Just a short comment, regarding the use of ALIEN_JDL_ADDTIMESERIESINMC: if we do this in anchorMC.sh, then it will concern only the anchored MC, while we want it for any MC (common, PWG?) right?

Isn't it:

  • Anchored MC: anchorMC.sh --> o2dpg_sim_workflow_anchored.py --> o2dpg_sim_workflow.py
  • Unanchored: o2dpg_sim_workflow.py

Couldn't this be done in the base script o2dpg_sim_workflow.py?

Thanks, Catalin.

@chiarazampolli
Copy link
Collaborator

Hello,

I don't think that this is needed in unanchored MCs. I would even think that unanchored is not needed at all anymore...

@miranov25, can you comment?

Chiara

@chiarazampolli
Copy link
Collaborator

Hello @benedikt-voelkel ,
Why do you need a default? I mean, even without it, it will be ON if you don't specify anything different.
Is this just more elegant? Or is there something I overlook?

Chiara

@benedikt-voelkel
Copy link
Contributor Author

benedikt-voelkel commented Feb 29, 2024

It's just for consistency. Of course it would not create problems but this relates somewhat to the "unbound issue" we had although - again - in this case, it would be fine.
But most importantly, I wanted to align things a little bit more and have everything that is somehow used at one place at the top of the file if that makes sense.
Otherwise one could also think that the variable is set somewhere in a sourced file earlier and appears magically.

@chiarazampolli
Copy link
Collaborator

Sure, fine with me, I wanted to make sure that I did not miss anything, since I tagged already without this change.

@miranov25
Copy link
Contributor

Hello,

I don't think that this is needed in unanchored MCs. I would even think that unanchored is not needed at all anymore...

@miranov25, can you comment?

Chiara

I am not sure I understand the comment. For unanchored MC, the time series are indeed useless, but representatively sampled data are useful (I was checking it)
If the unanchored production is still used in the physical analysis for corrections, I would like to keep the representative sampling. I will check what happened in the jet production which I was running at GSI.

@benedikt-voelkel
Copy link
Contributor Author

In any case, that is a different topic: For unanchored we would need to change different things.
Merging.

@benedikt-voelkel benedikt-voelkel merged commit b9cb67a into master Feb 29, 2024
11 checks passed
@benedikt-voelkel benedikt-voelkel deleted the benedikt-voelkel-patch-2 branch February 29, 2024 16:49
@chiarazampolli
Copy link
Collaborator

Unanchored should not be used for corrections, which is why i only applied the change to the anchoring script.

if needed, let’s take it in another pr.

Chiara

fcatalan92 pushed a commit to fcatalan92/O2DPG that referenced this pull request Apr 9, 2024
* help messages
* set ALIEN_JDL_ADDTIMESERIESINMC to a value at the beginning
noferini pushed a commit that referenced this pull request Apr 12, 2024
* help messages
* set ALIEN_JDL_ADDTIMESERIESINMC to a value at the beginning
benedikt-voelkel added a commit that referenced this pull request Apr 26, 2024
* help messages
* set ALIEN_JDL_ADDTIMESERIESINMC to a value at the beginning
benedikt-voelkel added a commit that referenced this pull request Apr 26, 2024
* help messages
* set ALIEN_JDL_ADDTIMESERIESINMC to a value at the beginning
chiarazampolli pushed a commit that referenced this pull request Aug 9, 2024
* help messages
* set ALIEN_JDL_ADDTIMESERIESINMC to a value at the beginning

(cherry picked from commit b9cb67a)
@chiarazampolli chiarazampolli added the async-2023-pp-apass4 Request porting to async-2023-pp-apass4 label Aug 9, 2024
@chiarazampolli
Copy link
Collaborator

Label async-2023-pp-apass4 added to make the needed PR (#1686) apply

@chiarazampolli chiarazampolli removed the async-2023-pp-apass4 Request porting to async-2023-pp-apass4 label Aug 9, 2024
@chiarazampolli
Copy link
Collaborator

Tag for async-2023-pp-apass4 created, removing label.

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

Successfully merging this pull request may close these issues.

4 participants