MiNoPy icon indicating copy to clipboard operation
MiNoPy copied to clipboard

MiaplPy suggestions 3rd round

Open falkamelung opened this issue 3 years ago • 7 comments

  • concatenate_patches (plural) as --start option

  • We currently have the minopy.interferograms.numSequential option. Let's switch to minopy.interferograms.connNum to be consistent with MintPy (it has mintpy.network.connNumMax). We also could switch both to numConn to be consistent wit --num_connections of ISCE -- up to you. In the paper we still can use sequential as does the MintPy paper.

  • There is a similar naming issue for minopy.inversion.stbas_numCon. (apart from changing numCon, stbas should probably change to sbw)

  • The network_inversion.py call is a bit weird. Should it not have an ifgramStack as argument and read the --template minopyApp.cfg (as done in MintPy's ifgram_inversion, see below)

network_inversion.py --template /work2/05861/tg851601/stampede2/codet/rsmas_insar/samples/unittestGalapagosSenDT128.template --work_dir /scratch/05861/tg851601/unittestGalapagosSenDT128/minopy/sequential_2_network
ifgram_inversion.py /scratch1/05861/tg851601/unittestGalapagosSenDT128/mintpy/inputs/ifgramStack.h5 -t /scratch1/05861/tg851601/unittestGalapagosSenDT128/mintpy/smallbaselineApp.cfg --update
  • I think it is better to write directory names other way around
single_reference_network --> network_single_reference
sequential-1_network --> network_sequential-1
  • The network selected for phase unwrapping shows up first in run_04_minopy_generate_ifgram_0. I think we should separate between run_files for phase linking and those for interferogram generation and unwrapping. We could either have run_files_single_reference or network_single_reference/run_files (I think I like the first)
  • can't we move inverted/interferograms_delaunay_1 to network_delaunay_1/interferograms?
  • Here two possible directory structures:
minopy/run_files
minopy/run_files_sequential-3
minopy/interferograms_sequential-3
minopy/mintpy_sequential-3

or

minopy/run_files
minopy/network_sequential-3/run_files
minopy/network_sequential-3/interferograms
minopy/network_sequential-3

I don't really have a preference. The advantage of mintpy_sequential-3 is that find exactly the same files as in the regular mintpy directory.

  • network.pdf is double:
rw-rw---- 1 tg851601 G-820134 13215 Mar 15 20:22 delaunay_1_network/network.pdf
-rw-rw---- 1 tg851601 G-820134 13215 Mar 15 20:22 network.pdf
  • For the ifgram_correction step the smallbaselinesApp.py command is not printed. For all(?) others it is displayed as the first command:

******************** step - ifgram_correction ********************
Generate /scratch/05861/tg851601/unittestGalapagosSenDT128/minopy/run_files/run_07_mintpy_ifgram_correction

___________________________________________________________

  /##      /## /##             /##     /#######           
 | ###    /###|__/            | ##    | ##__  ##          
 | ####  /#### /## /#######  /######  | ##  \ ## /##   /##
 | ## ##/## ##| ##| ##__  ##|_  ##_/  | #######/| ##  | ##
 | ##  ###| ##| ##| ##  \ ##  | ##    | ##____/ | ##  | ##
 | ##\  # | ##| ##| ##  | ##  | ## /##| ##      | ##  | ##
 | ## \/  | ##| ##| ##  | ##  |  ####/| ##      |  #######
 |__/     |__/|__/|__/  |__/   \___/  |__/       \____  ##
                                                 /##  | ##
                                                |  ######/
   Miami InSAR Time-series software in Python    \______/ 
          MintPy v1.3.2-42, 2022-03-09
___________________________________________________________

--RUN-at-2022-03-15 22:13:35.895314--
Current directory: /scratch/05861/tg851601/unittestGalapagosSenDT128/minopy
Run routine processing with smallbaselineApp.py on steps: ['modify_network', 'reference_point', 'quick_overview', 'correct_unwrap_error']

******************** step - invert_network ********************
Generate /scratch/05861/tg851601/unittestGalapagosSenDT128/minopy/run_files/run_08_minopy_invert_network
20220315:221355 * network_inversion.py --template /work2/05861/tg851601/stampede2/codet/rsmas_insar/samples/unittestGalapagosSenDT128.template --work_dir /scratch/05861/tg851601/unittestGalapagosSenDT128/minopy/sequential_2_network
All updated (removed) attributes already exists (do not exists) and have the same value, skip update.
  • In the minopyApp.cfg be a bit more verbose on steps 6,7. It is more than loading
########## 6,7. Load interferograms
# Set options in mintpy config file
  • I am not sure I like the name run_07_mintpy_ifgram_correction. If unwrap corrections is switched off we don't correct? What else could we use?

  • There is a newline missing in run_08_minopy_invert_network. It shows up as 0 lines

wc -l run_08_minopy_invert_network
0 run_08_minopy_invert_network
//login3/scratch/05861/tg851601/BristolDryLakeSenDT173/minopy/run_files[1035] cat run_08_minopy_invert_network
network_inversion.py --template /work2/05861/tg851601/stampede2/insarlab/infiles/famelung/TEMPLATES/BristolDryLakeSenDT173.template --work_dir /scratch/05861/tg851601/BristolDryLakeSenDT173/minopy/sequential_5_network//login3/scratch/05861/tg851601/BristolDryLakeSenDT173/minopy/run_files

falkamelung avatar Mar 13 '22 23:03 falkamelung

numConn and maxNumConn sounds more fluent to me as it's in the same order as "maximum number of connections". I may change mintpy option in the future to this.

yunjunz avatar Mar 14 '22 03:03 yunjunz

  • Apr 18: Installation on Mac using conda does not work because of gcc_linux (or similar ) in requirements.txt
  • from osgeo import goal in notebooks
  • middle of period for reference date default is not good. On northern hemisphere use first image in September and on southern hemisphere first image in March (snowfree and the hottest part of summer is over)
  • even if unwrapErrror correction is turned off it does this step. If this can't be avoided it should give a clear message so that it is clear to the user that things are working as expected. Also need to explain in detail in 'Discussion/Troubleshooting' section

falkamelung avatar Apr 18 '22 18:04 falkamelung

You could use c-complier as a platform-neutral name for gcc in conda-forge (https://github.com/conda-forge/compilers-feedstock). Similar names exists for cxx and fortran as well.

yunjunz avatar Apr 18 '22 19:04 yunjunz

The far I came some time ago was that if you do a conda install you end up with weird filenames. There is no file in my bin directory called gcc. Setting alias gcc=x86_64-conda_cos6-linux-gnu-gcc works. Also setting $CC environment variable (or similar) worked (I believe). Maybe Yunjun's link has some hints on how to handle the naming issue.

3rdparty/miniconda3/bin[1048] ls *gcc*
x86_64-conda_cos6-linux-gnu-gcc     

x86_64-conda_cos6-linux-gnu-gcc-nm
x86_64-conda_cos6-linux-gnu-gcc-ar  
x86_64-conda_cos6-linux-gnu-gcc-ranlib
x86_64-conda_cos6-linux-gnu-gcc --version
x86_64-conda_cos6-linux-gnu-gcc (crosstool-NG 1.23.0.449-a04d0) 7.3.0
Copyright (C) 2017 Free Software Foundation, Inc.

falkamelung avatar Apr 18 '22 19:04 falkamelung

After installing these compilers with conda, environment variables ${CC} and ${CXX} are created by default, to point to the correct files with long names.

yunjunz avatar Apr 18 '22 20:04 yunjunz

Thank you @yunjunz, that worked for me without error. @falkamelung would you test now? I committed a fix

mirzaees avatar Apr 18 '22 21:04 mirzaees

The code currently uses numProcessor. I think it should use minopy.multiprocessing.numCores. On Stampede 1 processor has 48 cores (and 96 CPUs).

falkamelung avatar Apr 30 '22 20:04 falkamelung