AngusWright / CosmoPipe

Pipeline for Cosmic Shear Cosmology
GNU General Public License v3.0
0 stars 2 forks source link

Patches #7

Open andrejdvornik opened 1 month ago

andrejdvornik commented 1 month ago

In a couple of scripts the looping through patches is done this way: for patch in @PATCHLIST@ @ALLPATCH@ @ALLPATCH@comb (or a variation of those variables). Now this usually works fine from the tests I have seen as it as a next step checks if the data that is requested exists given the patch designations, like this:

for file in ${headfiles}
do 
    if [[ "$file" =~ .*"_${patch}_".* ]] 
    then 
      filelist="${filelist} ${file}" 
    fi 
done

The bug / unexpected behaviour occurs when one passes the NS for the requested patch. This causes the script to unpack the variables in the following way: for patch in NS NS, which for a couple of scripts (namely calc stuff with treecorr) runs the same thing twice!

If one then requests the PATCHLIST=" ", the treecorr in this case doesn't run twice (as expected, as NS is the value set by ALLPATCH), but given that the input catalogues are added with add_main, which loops only over through PATCHLIST it just fails.

The solution I see is to re-set the PATCHLIST variable as one runs the pipeline, but that is quite counter productive. I think it should be checked somewhere that if user requests NS for the patch, it should make sure that option is passed through only once either through PATCHLIST or ALLPATCH

Could be I am missing something here though.

AngusWright commented 1 month ago

PATCHLIST should never be set to the same thing as ALLPATCH, because you're effectively creating a recursion. If you have them set to the same thing, then e.g. running combine_patch or extract_patch would corrupt the file, because it would be used as both input and output in the ldac processes. Neither variable is also not designed to be dynamically, because they define the input properties of the survey. If you want the code to treat e.g. the two KiDS hemispheres as one (which you shouldn't!!) then you should set ALLPATCH to something other than NS.

AngusWright commented 1 month ago

I will put in a catch that errors at compilation if ALLPATCH and PATCH are identical

andrejdvornik commented 1 month ago

Ok, I see that the ALLPATCH is actually set in the variables.sh, I missed it and assumed it always defaults to NS, which explains the issue actually. But a warning if those two variables are the same is welcome!