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

MiaplPy suggestions 3rd round #67

Open
falkamelung opened this issue Mar 13, 2022 · 7 comments
Open

MiaplPy suggestions 3rd round #67

falkamelung opened this issue Mar 13, 2022 · 7 comments

Comments

@falkamelung
Copy link
Member

falkamelung commented Mar 13, 2022

  • 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
@yunjunz
Copy link

yunjunz commented Mar 14, 2022

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.

@falkamelung
Copy link
Member Author

falkamelung commented Apr 18, 2022

  • 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

@yunjunz
Copy link

yunjunz commented Apr 18, 2022

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.

@falkamelung
Copy link
Member Author

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.

@yunjunz
Copy link

yunjunz commented Apr 18, 2022

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

@mirzaees
Copy link
Collaborator

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

@falkamelung
Copy link
Member Author

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

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

No branches or pull requests

3 participants