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

Adaptations for the release validation #134

Merged
merged 13 commits into from
May 21, 2018
Merged

Conversation

dberzano
Copy link
Contributor

This is to track the work we are doing for making AliDPG scripts compatible with the release validation.

@dberzano dberzano force-pushed the fix-things branch 3 times, most recently from 306a1a1 to 42576c9 Compare May 19, 2018 03:21
@@ -119,7 +119,7 @@ void main_AODtrain(Int_t merge=0)

PrintSettings();

if (merge || doCDBconnect) {
if ((merge || doCDBconnect) && !gSystem->Getenv("OCDB_PATH")) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since OCDB_PATH is not set for Grid jobs, nothing changes on the Grid.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi Dario,
on a hopefully short time scale, the macro AODtrainRawAndMC.C will replace the two macros AODtrain.C and AODtrainsim.C, in order to have the same code to produce AODs for raw data and MC.
So, I was thinking that to avoid problems in the future, it could be worth to apply this modification also to main_AODtrainRawAndMC.C.

Thanks!

Ciao, Francesco

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me update the PR. I have also noticed that AOD macros don't load the custom OCDB.

@@ -109,6 +109,9 @@ void main_recCPass0(const char *filename="raw.root",Int_t nevents=-1, const char
else {
// setup ocdb by custom (if any) or default settings
if (gSystem->AccessPathName("localOCDBaccessConfig.C", kFileExists)==0) {
TString envRunNumBuf = gSystem->Getenv("ALIEN_JDL_LPMRUNNUMBER");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Executed only if localOCDBaccessConfig.C is there, which is not the case on the Grid.

@@ -262,6 +262,35 @@ main()
echo "***********************" 2>&1 | tee -a ocdb.log
echo aliroot -b -q "makeOCDB.C($run, \"$ocdb\", \"$defaultOCDB\", $detectorBitsQualityFlag)" 2>&1 | tee -a ocdb.log
aliroot -b -q "makeOCDB.C($run, \"$ocdb\", \"$defaultOCDB\", $detectorBitsQualityFlag)" 2>&1 | tee -a ocdb.log
if [[ $ALIEN_JDL_CREATEOCDBARCHIVE ]]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Environment variable ALIEN_JDL_CREATEOCDBARCHIVE (which would be CreateOCDBArchive on a JDL) is not defined on the Grid, so this part should not affect Grid jobs either.

@@ -472,7 +501,11 @@ copyScripts()

extractFileNamesFromXMLCollection()
{
grep turl|sed 's|^.*turl\s*=\s*"\s*\([a-zA-Z]*://.*\.root\).*$|\1|g'
if [[ ! $ROOTSYS || ! $ALIDPG_ROOT ]]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part does affect Grid jobs. As a matter of fact, we've replaced a, er, "homemade" XML parser with a proper one, see the new readAliEnCollection.sh script. The reason for this replacement is that it misbehaved for generic URLs.

We have tested it successfully: the parser is used if ROOT and AliDPG are both available in the job's environment (which should always be the case), otherwise we fall back to the old method.

@@ -86,6 +86,9 @@ void main_recCPass1(const char *filename="raw.root",Int_t nevents=-1, const char
else {
// setup ocdb by custom (if any) or default settings
if (gSystem->AccessPathName("localOCDBaccessConfig.C", kFileExists)==0) {
TString envRunNumBuf = gSystem->Getenv("ALIEN_JDL_LPMRUNNUMBER");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as for CPass0: does not affect Grid jobs.

@@ -261,6 +261,35 @@ main()
echo "***********************" 2>&1 | tee -a ocdb.log
echo aliroot -b -q "makeOCDB.C($run, \"$ocdb\", \"$defaultOCDB\", $detectorBitsQualityFlag)" 2>&1 | tee -a ocdb.log
aliroot -b -q "makeOCDB.C($run, \"$ocdb\", \"$defaultOCDB\", $detectorBitsQualityFlag)" 2>&1 | tee -a ocdb.log
if [[ $ALIEN_JDL_CREATEOCDBARCHIVE ]]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as for CPass0: does not affect Grid jobs.

@@ -470,7 +499,11 @@ copyScripts()

extractFileNamesFromXMLCollection()
{
grep turl|sed 's|^.*turl\s*=\s*"\s*\([a-zA-Z]*://.*\.root\).*$|\1|g'
if [[ ! $ROOTSYS || ! $ALIDPG_ROOT ]]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as for CPass0: this part does affect Grid jobs but it should work.

@@ -0,0 +1,18 @@
void readAliEnCollection(const char *in, const char *out) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the new XML parser. It is based on ROOT. We always build our ROOT with XML support, so this will work everywhere, including the Grid.

@@ -0,0 +1,7 @@
#!/bin/bash
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This script merely invokes the ROOT-based XML parser.

@@ -1,11 +1,14 @@
void merge(const char *filesToMerge="FilterEvents_Trees.root")
void merge(const char *filesToMerge="FilterEvents_Trees.root", const char *inputCollection="wn.xml")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change introduces an argument to the merge function, allowing to specify a different XML collection. Since Grid JDLs don't use the second parameter, they will always fall back to the default one, wn.xml. This part does affect Grid jobs but it was tested to work.

if (!gGrid || !gGrid->IsConnected()) {
::Error("QAtrain", "No grid connection");
return;
TString noGridConnect = gSystem->Getenv("ALIEN_JDL_NOGRIDCONNECT");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That envvar is not defined for Grid jobs, so this section does not affect them.

@@ -181,6 +179,12 @@ void CopyCPass(const char* alienFileList, const char* outputFileList, Int_t time
TString src("");
src.ReadLine(inputFile);
if (src.IsNull()) continue;
if (!counter && !gGrid) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Conditionally connect to the Grid. Grid input files are always alien://... so it should work there.

@@ -8,6 +8,14 @@ void main_rec(const char *filename="raw.root", const char* options="")


AliReconstruction rec;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have noticed that current reconstruction macros for PPass do not allow to reconstruct only part of the chunk (unlike CPass0 and 1). This variable (not used on the Grid) adds this capability consistently for every reconstruction job.

// setup ocdb by custom (if any) or default settings
if (gSystem->AccessPathName("OCDBconfig.C", kFileExists)==0) {
gROOT->ProcessLine("OCDBconfig.C");
}
else if (gSystem->AccessPathName("localOCDBaccessConfig.C", kFileExists)==0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the override OCDB macro is there, we load it. This does not affect Grid jobs.

@@ -89,6 +89,36 @@ if [[ ! -f "$locMacro" ]] ; then cp $macroName ./ ; fi

time aliroot -b -q "${locMacro}(\"$inputFileList\", $startRun, $endRun, \"$targetOCDBDir\",$corr ,$dist)" >> ocdb.log

if [[ $ALIEN_JDL_CREATEOCDBARCHIVE ]]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above. Does not affect Grid jobs.

@@ -81,6 +88,15 @@ void procResidData(int mode, // processing mode
//
}

//________________________________________________________________________
void loadLocalOCDBIfExists(AliTPCDcalibRes *clb) {
if (gSystem->AccessPathName("localOCDBaccessConfig.C", kFileExists) == 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part, again, does not affect Grid jobs.

@@ -69,7 +69,7 @@ extractEnvVars()
[[ $# -lt 3 ]] && Usage && exit 1
echo "Arguments: 1 = $1, 2 = $2, 3 = $3"

source $ALICE_PHYSICS/PWGPP/scripts/alilog4bash.sh
[[ -e $ALICE_PHYSICS/PWGPP/scripts/alilog4bash.sh ]] && source "$ALICE_PHYSICS"/PWGPP/scripts/alilog4bash.sh || source "$ALICE_ROOT"/libexec/alilog4bash.sh
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes a problem never noticed on the Grid: that file was moved and TPC SP jobs haven't had any log output since a while.

@@ -146,7 +146,7 @@ void main_QAtrain_duo(const char *suffix="", Int_t run = 0,
PrintSettings();

TString cdbString(cdb);
if (cdbString.Contains("raw://"))
if (cdbString.Contains("raw://") && !gSystem->Getenv("OCDB_PATH"))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

OCDB_PATH not set on the Grid, does not affect Grid jobs.

@@ -51,7 +51,7 @@ fi

if [ -f "QAtrain_duo.C" ]; then
echo "QAtrain_duo macro found"
if grep -q -E -e "/cpass1|cpass1/" *.xml; then
if grep -q -E -e "/cpass1|cpass1/" *.xml || grep -q -E -e "/cpass1|cpass1/" "$1"; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does not affect Grid jobs, where input collection is always a XML file. It makes it work for text files too, in the release validation case (used if we don't put our files in zip archives).

@dberzano
Copy link
Contributor Author

Hello @chiarazampolli, I would merge it too. I have added comments to the code on this PR to better clarify what parts affect/do not affect Grid jobs. Parts that "affect" Grid jobs should be harmless, or fix unnoticed misbehaviours. Those comments will be helpful in tracking potential mishaps during the next reconstruction. Nothing bad should happen, but you can never tell 😄

@fprino
Copy link
Collaborator

fprino commented May 19, 2018

Hi,

thanks for the work. I agree to proceed merging the pull request.
I added a comment about the AOD macro, but that modification can be done also at a later stage.

Cheers, Francesco

@dberzano
Copy link
Contributor Author

@fprino I have done the requested modification in AODtrainRawAndMC, so we're more future-proof. I think you can merge if you agree, and backport to the v5-09-XX branch too.

@fprino fprino merged commit d36d2a2 into alisw:master May 21, 2018
@fprino
Copy link
Collaborator

fprino commented May 21, 2018

Hi Dario, Matteo,

thanks for the work and for the additional modification for the AOD filtering.
I have merged the PR.

Ciao, Francesco

@chiarazampolli
Copy link
Collaborator

Ciao,

(sorry for the delay, I was in a working session with a colleague till now).

I want also to thank a lot Dario and Matteo for the huge work!!

Chiara

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.

3 participants