making stackSentinel.py faster

[hardreddata]

Hi,

I have been using stackSentinel from https://github.com/isce-framework/isce2/blob/main/contrib/stack/topsStack/README.md

I noticed that most of the time was in the Snaphu phase unwrapping and that it wasn't especially CPU intensive.

I had good outcomes manually editing the shell script to chunk runs into batches. Here batches of four.

# lots more chunks prior to this
SentinelWrapper.py -c /mnt/i/stack/configs/config_igram_unw_20201031_20201218 &
SentinelWrapper.py -c /mnt/i/stack/configs/config_igram_unw_20201031_20201230 &
SentinelWrapper.py -c /mnt/i/stack/configs/config_igram_unw_20201112_20201124 &
SentinelWrapper.py -c /mnt/i/stack/configs/config_igram_unw_20201112_20201206 &
wait

SentinelWrapper.py -c /mnt/i/stack/configs/config_igram_unw_20201112_20201218 &
SentinelWrapper.py -c /mnt/i/stack/configs/config_igram_unw_20201112_20201230 &
SentinelWrapper.py -c /mnt/i/stack/configs/config_igram_unw_20201112_20210111 &
SentinelWrapper.py -c /mnt/i/stack/configs/config_igram_unw_20201124_20201206 &
wait

I have modified Stack.py to accomplish this from the --num-process 4 command line argument. If this is of interest please let me know and I can do a pull request. Feedback on other steps which are time consuming and could benefit is invited. I get that not all steps can be split apart into independent chunks.

Changes are in here

main...RussellGrew:crude-num-process

Thanks for making this great resource available open source.

[rtburns-jpl]

Thanks! Looks like a nice improvement. I'd say go ahead and submit a pull request; I'll ask if anyone else has comments/suggestions.

[krasny2k5]

Very nice feature! I was thinking in doing exactly this after checking that some steps was not very CPU intensive. I'll check this and report if I find any problem.

[piyushrpt]

SNAPHU runs on one core but is RAM hungry. How many unwrapping jobs one is able to run in parallel, really depends on how big the machine is. Might be better to make this an option in setting up the run files rather than hardcoding it.

[hardreddata]

Thanks for the promising feedback.

@piyushrpt yes this is an option you can pass to stackSentinel.py with the --num-process argument.

I realised I had an oversight with the default num-process (ie: 1) whereby all the modified shell script commands would go to the background via & and attempt to run at once.

This

self.runf.write(self.text_cmd + 'SentinelWrapper.py -c ' + configName + ' &\n')
if self.numProcess > 1 and line_cnt == self.numProcess:
    self.runf.write('wait\n\n')
    line_cnt = 0

It might be better as the below. Probably as its own function and not repeated throughout Stack.py as I have now.

if self.numProcess == 1:
    self.runf.write(self.text_cmd + 'SentinelWrapper.py -c ' + configName + '\n')
elif self.numProcess > 1:
    self.runf.write(self.text_cmd + 'SentinelWrapper.py -c ' + configName + ' &\n')
    if line_cnt == self.numProcess:
        self.runf.write('wait\n\n')
        line_cnt = 0

Cheers.

[rtburns-jpl]

Alternatively, how about removing the self.numProcess > 1 check? Then if numProcess == 1, there will be a wait after every line and you won't need the special case.

[hardreddata]

For reference I typically run the first step by itself sh run_01_unpack_topo_reference to ensure the DEM and orbits and command line arguments are all happy, and then I daisy chain the rest together:

sh run_02_unpack_secondary_slc && sh run_03_average_baseline && sh run_04_extract_burst_overlaps && sh run_05_overlap_geo2rdr && sh run_06_overlap_resample && sh run_07_pairs_misreg && sh run_08_timeseries_misreg && sh run_09_fullBurst_geo2rdr && sh run_10_fullBurst_resample && sh run_11_extract_stack_valid_region && sh run_12_merge_reference_secondary_slc && sh run_13_generate_burst_igram && sh run_14_merge_burst_igram && sh run_15_filter_coherence && sh run_16_unwrap

run_16_unwrap will finish and prompt block and I hit Enter.

@rtburns-jpl if you are happy to have the & and wait throughout the bash scripts then this sounds like a good idea.

[yunjunz]

This change could essentially eliminate the use of topsStack/run.py and is easier, which is great!

I am wondering what is the impact of & and independent lines of wait in the run_file for people who use job schedulers on HPC? Since each line is one independent job with the current run file structure if I understand it right. @mirzaees what do you think?

[mirzaees]

@yunjunz, It depends very much on how you setup your job on HPC,

It works and might be a good option on some machines but on TACC system (stampede2 and frontera) specifically, they suggest to use 'launcher' for parallel submission of tasks in each run file and it does not require '&' and 'wait'
launcher would manage the distribution of resources to each task. I have tried both and I prefer to use launcher

I agree with @piyushrpt that it should be optional

[hardreddata]

Thanks @yunjunz and @mirzaees for your review.

@rtburns-jpl I have just updated the code at 419b0e6 to leave the single process case untouched.

I am more end user than developer. Feedback on the nature of the solution is very welcome. I don't think this function could be included in the existing classes.

I thought about calling it whenever we have self.text_cmd + 'SentinelWrapper.py -c ' + configName + '\n' but this may be too gratuitous.

Advice invited.

[yunjunz]

Thank you @RussellGrew for the update. Calling self.text_cmd + 'SentinelWrapper.py -c ' + configName whenever possible gives a consistent style and sounds like a good idea to me.

[hardreddata]

#225 updated

[hardreddata]

I think we are done here. Thanks for being receptive to the idea.