geodesymiami / rsmas_insar

RSMAS InSAR code
https://rsmas-insar.readthedocs.io/
GNU General Public License v3.0
58 stars 22 forks source link

Bug: create_runfiles.py creates copy_to_tmp code without --tmp option #501

Closed falkamelung closed 2 years ago

falkamelung commented 2 years ago

Hi @Ovec8hkin : When I run

create_runfiles.py /home1/05861/tg851601/code/rsmas_insar/samples/unittestGalapagosSenDT128.template --jobfiles --queue flex

it creates the lines

################################################
#   install code on /tmp                       #
################################################
install_to_tmp.bash /scratch1/05861/tg851601/unittestGalapagosSenDT128/run_files/run_02_unpack_secondary_slc_0.job --prefix tops
copy_to_tmp.bash /scratch1/05861/tg851601/unittestGalapagosSenDT128/run_files/run_02_unpack_secondary_slc_0.job /scratch1/05861/tg851601/unittestGalapagosSenDT128/run_files/run_02_unpack_secondary_slc_0 /scratch1/05861/tg851601/unittestGalapagosSenDT128
# set environment    
export RSMASINSAR_HOME=/tmp/rsmas_insar
cd $RSMASINSAR_HOME; source ~/accounts/platforms_defaults.bash; source setup/environment.bash; export PATH=$ISCE_STACK/topsStack:$PATH; cd -;

in the *job file. It should not. They should be created only for --tmp option.

Ovec8hkin commented 2 years ago

To clarify, you don't need any of those lines in the job file if the --tmp flag is not set? What about the contents of install_to_tmp.bash? Does any of that code need to be written to the job file if --tmp is not used?

falkamelung commented 2 years ago

Correct. These lines need to be omitted if --tmp is not set. There is nothing to do with the contents of install_to_tmp.bash. It will use the code as given on the path.

Ovec8hkin commented 2 years ago

Ok, I misunderstood the initial request. Should be an easy fix. I am waiting for data to download right now so I can test.

falkamelung commented 2 years ago

And when you work on this, can you add --help options to copy_to_tmp.bash and to install_to_tmp.bash? Just write whatever you know. I can continue from there. I don't understand why copy_to_tmp.bash requires 3 arguments.

copy_to_tmp.bash /scratch/05861/tg851601/unittestGalapagosSenDT128/run_files_tmp/run_03_average_baseline_0.job /scratch/05861/tg851601/unittestGalapagosSenDT128/run_files_tmp/run_03_average_baseline_0 /scratch/05861/tg851601/unittestGalapagosSenDT128
Ovec8hkin commented 2 years ago

I found out what the problem is here. Someone set a new field in job_defaults.cfg called 'copy_to_tmp' and set the default value to 'yes' for everything. Is 'yes' the desired default, or should they be changed to 'no'?

falkamelung commented 2 years ago

I added this a while ago. Before you started with copy_to_tmp, I thought ??

It should be all yes. But I plan to set run_08 step (and possibly others) to no. This step takes 15 minutes for copy_to_tmp.bash but only 1 minute without copy_to_tmp. I plan to limit to 50 simultaneous tasks for this step (and more if TACC allows). This step is the bottleneck. I expect ~50% speed-up for an entire workflow.

So we need the ability to switch off copy_to_tmp for individual steps. We had this already. Only I did not use it.

Not using copy_to_tmp or install_to_tmp is for platforms different then Stampede and Frontera that don't have /tmp.

Ovec8hkin commented 2 years ago

If you modify the 'job_defaults.cfg' entry for 'copy_to_tmp' to 'no' it works as desired. I don't see what the issue is.

falkamelung commented 2 years ago

Yes. This works.

The issue was to have the ability to create with create_runfiles.py without any copy_to_tmp stuff. Maybe that was not clear?

It actually would be good to have the option to run

create_runfiles.py --tmp
create_runfiles.py 
create_runfiles.py --no_tmp

The last two would do exactly the same.

Ovec8hkin commented 2 years ago

Right. If you don't want the copy_to_tmp stuff when set the job_defaults.cfg option value to no.

falkamelung commented 2 years ago

This is what create_runfiles.py --no_tmp also should do in a much easier way. Nobody should have to modify job_defaults.cfg.

On Expanse at UCSD we don't need copy_to_tmp, but I don't have an easy way to try this platform. At a later stage we may want to switch copy_to_tmp on/off in the queues.cfg

Ovec8hkin commented 2 years ago

So you want --no_tmp to act as a universal override turning off tmp for all steps regardless of the job_defaults.copy_to_tmp option?

falkamelung commented 2 years ago

Yes, exactly. And no option would do the same as --no_tmp. All what it would do is not to write the lines above into the jobfile

Ovec8hkin commented 2 years ago

Ok. I understand now. Do you want an explicit --no_tmp option available to use, or is just not supplying any option sufficient?

falkamelung commented 2 years ago

Yes. explicit. I already added --no_tmp to minsarApp.bash and run_workflow.bash. Else it is confusing.

Ovec8hkin commented 2 years ago

Just pushed. Should be as expected now.

falkamelung commented 2 years ago

Thank you. That works nicely now! We can easily run with and without copy_to_tmp. Only there is a newline missing after the walltime (see below). It works but is ugly. I could not find easily where this is.

#SBATCH -p flex
#SBATCH -t 0:02:42################################################
# execute tasks with launcher
################################################
export OMP_NUM_THREADS=2
falkamelung commented 2 years ago

Sara fixed this. So this is done.