geocode SLC integration test #46

LiangJYu · 2022-08-09T16:30:32Z

This PR adds a unit test for geocode SLC and associated test data.

To Do:

upload unit test data Zenodo
add ability to cache and download data

vbrancat

@LiangJYu thanks for this PR. I have not tested it yet. Do we have the capability to run the using ctest or are we planning to have this running only on the CI system?

tests/test_s1_geocode_slc.py

tests/conftest.py

LiangJYu · 2022-09-12T22:50:49Z

@LiangJYu thanks for this PR. I have not tested it yet. Do we have the capability to run the using ctest or are we planning to have this running only on the CI system?

It works with pytest locally. To test for yourself, do the following:

go to COMPASS source directory
type pytest

I need to check if I've integrated this with CI. Thank you for bringing that to my attention.

LiangJYu · 2022-09-14T00:33:04Z

@rtburns-jpl Based on this, I was thinking about making the following additions to enable this unit test circleci yaml? Am I on the right track?

docker/Docker

FROM build AS test
RUN pytest

circleci/config.yml under jobs/build/steps:

      - run:
          name: "Run unit tests"
          command: |
            docker build . --target test

rtburns-jpl · 2022-09-14T00:40:06Z

Yeah, I think that should work. You could also try running pytest from a docker run command, then you won't need to add any steps to the dockerfile.

rtburns-jpl · 2022-11-01T20:26:44Z

The docker command docker run conda activate COMPASS is trying to run the command activate COMPASS inside an image named conda. So I think what you want to do is tag the image in the docker build command, e.g. -t mytag and then do docker run mytag <...> so your command runs inside that image.

.circleci/config.yml

seongsujeong

Thanks for adding the test module for COMPASS. Here are couple of things I've found after testing it on my end:

After testing it on my machine, I've worked a bit on the Docker command and the Dockerfile. Please find my suggestions below.
I've found a small depe,dency issue as below:

Traceback (most recent call last):
  File "/home/compass_user/miniconda3/envs/COMPASS/lib/python3.9/runpy.py", line 188, in _run_module_as_main
    mod_name, mod_spec, code = _get_module_details(mod_name, _Error)
  File "/home/compass_user/miniconda3/envs/COMPASS/lib/python3.9/runpy.py", line 147, in _get_module_details
    return _get_module_details(pkg_main_name, error)
  File "/home/compass_user/miniconda3/envs/COMPASS/lib/python3.9/runpy.py", line 111, in _get_module_details
    __import__(pkg_name)
  File "/home/compass_user/miniconda3/envs/COMPASS/lib/python3.9/site-packages/pytest/__init__.py", line 5, in <module>
    from _pytest._code import ExceptionInfo
  File "/home/compass_user/miniconda3/envs/COMPASS/lib/python3.9/site-packages/_pytest/_code/__init__.py", line 2, in <module>
    from .code import Code
  File "/home/compass_user/miniconda3/envs/COMPASS/lib/python3.9/site-packages/_pytest/_code/code.py", line 35, in <module>
    import attr
ModuleNotFoundError: No module named 'attr'

Installing attr with conda from conda-forge solved the issue. Below is the explicit dependency list after that. I suggest to update specifile with these.

# This file may be used to create an environment using:
# $ conda create --name <env> --file <this file>
# platform: linux-64
@EXPLICIT
https://conda.anaconda.org/conda-forge/linux-64/_libgcc_mutex-0.1-conda_forge.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/ca-certificates-2022.9.24-ha878542_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/font-ttf-dejavu-sans-mono-2.37-hab24e00_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/font-ttf-inconsolata-3.000-h77eed37_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/font-ttf-source-code-pro-2.038-h77eed37_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/font-ttf-ubuntu-0.83-hab24e00_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/ld_impl_linux-64-2.36.1-hea4e1c9_2.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libgfortran5-12.1.0-hdcd56e2_16.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libstdcxx-ng-12.1.0-ha89aaad_16.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/poppler-data-0.4.11-hd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/tzdata-2022a-h191b570_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/fonts-conda-forge-1-0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libgfortran-ng-12.1.0-h69a702a_16.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libgomp-12.1.0-h8d9b700_16.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/_openmp_mutex-4.5-2_gnu.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/fonts-conda-ecosystem-1-0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libgcc-ng-12.1.0-h8d9b700_16.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/attr-2.5.1-h166bdaf_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/bzip2-1.0.8-h7f98852_4.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/c-ares-1.18.1-h7f98852_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/expat-2.4.8-h27087fc_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/fftw-3.3.10-nompi_h77c792f_102.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/freexl-1.0.6-h7f98852_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/geos-3.10.2-h9c3ff4c_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/giflib-5.2.1-h36c2ea0_2.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/icu-69.1-h9c3ff4c_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/jpeg-9e-h166bdaf_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/json-c-0.16-hc379101_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/keyutils-1.6.1-h166bdaf_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/lerc-3.0-h9c3ff4c_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libdeflate-1.10-h7f98852_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libedit-3.1.20191231-he28a2e2_2.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libev-4.33-h516909a_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libffi-3.4.2-h7f98852_5.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libiconv-1.16-h516909a_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libnsl-2.0.0-h7f98852_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libopenblas-0.3.20-pthreads_h78a6416_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libuuid-2.32.1-h7f98852_1000.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libwebp-base-1.2.2-h7f98852_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libzlib-1.2.12-h166bdaf_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/lz4-c-1.9.3-h9c3ff4c_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/ncurses-6.3-h27087fc_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/nspr-4.32-h9c3ff4c_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/openssl-1.1.1s-h166bdaf_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/pcre-8.45-h9c3ff4c_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/pixman-0.40.0-h36c2ea0_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/pthread-stubs-0.4-h36c2ea0_1001.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/readline-8.1-h46c0cb4_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/snappy-1.1.9-hbd366e4_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/tzcode-2022a-h166bdaf_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/xorg-kbproto-1.0.7-h7f98852_1002.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/xorg-libice-1.0.10-h7f98852_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/xorg-libxau-1.0.9-h7f98852_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/xorg-libxdmcp-1.1.3-h7f98852_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/xorg-renderproto-0.11.1-h7f98852_1002.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/xorg-xextproto-7.3.0-h7f98852_1002.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/xorg-xproto-7.0.31-h7f98852_1007.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/xz-5.2.5-h516909a_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/yaml-0.2.5-h7f98852_2.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/gettext-0.19.8.1-h73d1719_1008.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/krb5-1.19.3-h3790be6_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libblas-3.9.0-15_linux64_openblas.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/librttopo-1.1.0-hf69c175_9.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libxcb-1.13-h7f98852_1004.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/tk-8.6.12-h27826a3_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/xerces-c-3.2.3-h8ce2273_4.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/xorg-libsm-1.2.3-hd9c2040_1000.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/zlib-1.2.12-h166bdaf_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/zstd-1.5.2-h8a70e8d_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/blosc-1.21.1-h83bc5f7_3.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/boost-cpp-1.74.0-h6cacc03_7.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/hdf4-4.2.15-h10796ff_3.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libcblas-3.9.0-15_linux64_openblas.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libglib-2.70.2-h174f98d_4.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/liblapack-3.9.0-15_linux64_openblas.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libnghttp2-1.47.0-h727a467_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libpng-1.6.37-h21135ba_2.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libpq-14.2-hd57d9b9_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libssh2-1.10.0-ha56f1ee_2.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libtiff-4.4.0-h0fcbabc_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libxml2-2.9.12-h885dcf4_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libzip-1.8.0-h4de3113_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/sqlite-3.38.5-h4ff8645_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/xorg-libx11-1.7.2-h7f98852_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/freetype-2.10.4-h0708190_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/lcms2-2.12-hddcbb42_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libcurl-7.83.1-h7bff187_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libkml-1.3.0-h238a007_1014.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/nss-3.78-h2350873_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/openjpeg-2.4.0-hb52868f_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/postgresql-14.2-h2510834_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/python-3.9.13-h9a8a25e_0_cpython.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/xorg-libxext-1.3.4-h7f98852_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/xorg-libxrender-0.9.10-h7f98852_1003.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/attrs-22.1.0-pyh71513ae_1.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/backoff-2.1.0-pyhd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/cached_property-1.5.2-pyha770c72_1.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/certifi-2022.9.24-pyhd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/cfitsio-4.1.0-hd9d235c_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/charset-normalizer-2.0.12-pyhd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/colorama-0.4.4-pyh9f0ad1d_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/curl-7.83.1-h7bff187_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/exceptiongroup-1.0.4-pyhd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/fontconfig-2.14.0-h8e229c2_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/hdf5-1.12.1-nompi_h2386368_104.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/idna-3.3-pyhd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/iniconfig-1.1.1-pyh9f0ad1d_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/pluggy-1.0.0-pyhd8ed1ab_5.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/proj-9.0.0-h93bde94_1.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/pycparser-2.21-pyhd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/pyparsing-3.0.9-pyhd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/python_abi-3.9-2_cp39.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/six-1.16.0-pyh6c4a22f_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/tomli-2.0.1-pyhd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/wheel-0.37.1-pyhd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/cached-property-1.5.2-hd8ed1ab_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/cairo-1.16.0-ha12eb4b_1010.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/cffi-1.15.0-py39h4bc2ebd_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/geotiff-1.7.1-h509b78c_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/kealib-1.4.14-hfe1a663_4.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libdap4-3.20.6-hd7c4107_2.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libnetcdf-4.8.1-nompi_h329d8a1_102.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libspatialite-5.0.1-ha867d66_15.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/numpy-1.22.4-py39hc58783e_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/packaging-21.3-pyhd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/pycosat-0.6.3-py39hb9d737c_1010.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/pyre-1.11.2-py39h8d5ab89_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/pysocks-1.7.1-py39hf3d152e_5.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/pyyaml-6.0-py39hb9d737c_4.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/ruamel.yaml.clib-0.2.6-py39hb9d737c_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/ruamel_yaml-0.15.80-py39hb9d737c_1007.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/setuptools-62.3.2-py39hf3d152e_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/tiledb-2.8.3-h1e4a385_1.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/tqdm-4.64.0-pyhd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/brotlipy-0.7.0-py39hb9d737c_1004.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/conda-package-handling-1.8.1-py39hb9d737c_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/cryptography-37.0.2-py39hd97740a_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/h5py-3.6.0-nompi_py39h7e08c79_100.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/pip-22.1.2-pyhd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/poppler-22.04.0-h1434ded_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/pytest-7.2.0-py39hf3d152e_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/ruamel.yaml-0.17.21-py39hb9d737c_1.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/scipy-1.8.1-py39he49c0e8_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/shapely-1.8.2-py39h73b9895_1.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/yamale-4.0.4-pyh6c4a22f_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/libgdal-3.4.3-h56144a5_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/pyopenssl-22.0.0-pyhd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/gdal-3.4.3-py39hc691d54_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/urllib3-1.26.9-pyhd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/isce3-0.6.0-py39h73072f7_0.tar.bz2
https://conda.anaconda.org/conda-forge/noarch/requests-2.27.1-pyhd8ed1ab_0.tar.bz2
https://conda.anaconda.org/conda-forge/linux-64/conda-4.13.0-py39hf3d152e_1.tar.bz2

.circleci/config.yml

docker/Dockerfile

LiangJYu · 2022-11-19T00:22:38Z

@seongsujeong Thank you for helping with debugging! I guess order of the import matters because added attr in dbf457b and 7e34b3f.

update test data URL update validation to check for presense of reflectors

fix test file paths update test data URL

add environment yaml to help create clean/minimal environment add bounds check to margin application

scottstanie

My main lingering question- do we want this to run on every PR? It takes about 100 seconds total with the download, but only ~20-30 seconds once it's downloaded.

scottstanie · 2023-02-25T17:42:08Z

tests/test_s1_geocode_slc.py

+    return np.s_[i_max_y:i_min_y, i_min_x:i_max_x]
+
+
+def test_geocode_slc_validate(geocode_slc_params):


Does this test implicitly rely on test_geocode_slc_run having already run?

If so, maybe we should make test_geocode_slc_run into a fixture that returns something (like the output path maybe), and have test_geocode_slc_validate use that fixture.
Otherwise this would fail it we decide to add in a testing speed up like https://github.com/pytest-dev/pytest-xdist

By default it looks like tests are run in the order they are found (sorry I wasn't able to find anything definitive in the pytest docs). Anecdotally, swapping the order to order of the tests in code supports this.

Test order can be set if it becomes an issue.

Making test_geocode_slc_run a fixture would guarantee order but I think would remove test_geocode_slc_run as a test.

pytest-xdist issues would be tricky. I say we address those issues when we encounter them. I speculate that the runtime of tests calling isce3.geocode.geocode_slc will significantly be longer then any of the follow up (likely single-threaded) validation tests. The multi-threaded nature isce3.geocode.geocode_slc precludes them from being distributed like the single-threaded validation tests and that would diminish the returns of from parallelization.

I would prefer to have 2 separate tests. 1) testing that the actual workflow runs 2) validate the geolocation after running. Can we separate those two tests into separate functions?

tests/test_s1_geocode_slc.py

tests/conftest.py

scottstanie · 2023-02-25T17:54:20Z

tests/test_s1_geocode_slc.py

+    corner_reflector_threshold = 3e3
+    assert np.any(arr > corner_reflector_threshold)


Just checking- do you know how many normal, non-corner reflector pixels exceed this threshold?

IIRC 2 or 3 of the reflectors listed do not show up in the geocoded raster and I didn't want to figure where they were. As a first cut, I think a bare minimum "any reflectors in the expected slice" is sufficient. When all the corrections are implemented, I think the test can then be modified to check for individual reflectors.

scottstanie · 2023-02-25T17:58:40Z

compass_env.yml

+  - pandas
+  - pyproj
+  - pytest
+  - python>=3.9


have we use any 3.9+ specific features that would make 3.7/3.8 break?

So far no 🤞

Do we have a dependency on lxml? I thought the s1-reader had this dependency

I actually found that s1reader has just 2-3 cases where it type hinted using the list[thing] syntax introduced in 3.9.... personally I think we could just replace those with from typing import List; List[...] so we dont lose 3.8 compatibility unnecessarily, but that's a s1reader PR.

tests/conftest.py

scottstanie · 2023-02-25T18:31:08Z

.gitignore

+tests/product
+tests/scratch


If these are getting created in the test directory and we want to ignore them, what if we use a temporary pytest directory that'll get removed?

E.g. in conftest.py, using https://docs.pytest.org/en/7.1.x/how-to/tmp_path.html#the-tmp-path-factory-fixture

@pytest.fixture(scope="session") def geocode_slc_params(tmp_path_factory): ... # get test working directory test_path = pathlib.Path(__file__).parent.resolve() output_path = tmp_path_factory.mktemp("output") ... cfg = f_template.read().replace( '@TEST_PATH@', str(output_path)).\

But now that i've written this out, I'm thinking you may have stored it in the tests/ directory so that you could inspect the output, so feel free to ignore this suggestion if that's the case

Your reasoning for the scratch directory is spot on.

tempfile.TemporaryDirectory could be used just for unit tests but would disable ability to check intermediate results.

scottstanie · 2023-02-25T18:33:36Z

compass_env.yml

@@ -0,0 +1,19 @@
+name: compass


Do we need to make a separate conda environment file?

I did this in dolphin here https://github.com/opera-adt/dolphin/blob/main/conda-env.yml but my reasoning was that I had a python version in there, and that fails if you do pip install -r requirements.txt.

We have a requirements files that also has python, so right now you could only use that with conda.

Also, if we were to add this conda env for easier installation, should we add isce3 to it? Seems like we don't want to tell every person they should install it from source like the comment in requirements.txt says.

I added this because I forgot to add -c conda-forge to my conda create command and figured that out just now. 🤣 Having two files is redudant and I think the yaml should replace the requirements.txt because the contents of the yaml is more informative and avoids the mistake I made.

I agree with you on isce3 item. We could have two environment files with a lot of identical content; one for devs (no isce3) and one for just running COMPASS (with isce3). Or should there by just one environment file with separate info for ISCE3+COMPASS devs on how to overwrite isce3 when they build/install.

@scottstanie What do you think?

I'm still figuring out if there's any benefit to having the pip install option (where it's usual to have a requirements.txt file that you can link to in your package metadata file) in addition to the conda env file...

But I agree that just one file is probably be better.
Also,

yes I think you should have isce3 on there

for developers, which we assume are more sophisticated than average if they're trying to get dev isce3 working, they can uninstall the conda one/ go through the build with separate instructions. no need for a separate environment.yaml file with one fewer thing in it.

as a nitpick, I might go for environment.yaml as the file name or else conda-env.yaml to be even more explicit it's for conda. That lets someone know from looking at the file it's associated with conda, since the filename compass_env.yaml could be anything

I will remove the environment yaml file as it can only be used to create a new environment file. While being slightly more verbose to use, requirements.txt can be used to create a new environment and install packages to an existing one.

Co-authored-by: Scott Staniewicz <[email protected]>

…nto geocode_slc_unit_test

vbrancat

Left some comments on the CSLC validation. It could be nice to check whether our new code does not create any issue on the geolocation. More detailed comments to follow. Let's discuss the test design also in our CSLC-S1 meeting tag-up

vbrancat · 2023-02-28T19:25:01Z

compass_env.yml

+  - pandas
+  - pyproj
+  - pytest
+  - python>=3.9


Do we have a dependency on lxml? I thought the s1-reader had this dependency

vbrancat · 2023-02-28T19:25:31Z

requirements.txt

 gdal>=3
 #isce3        # since the conda-installed isce3 is not the most updated version, installing isce3 from stratch is recommended, to stay in sync with isce3 development.
 #journal      # as of Mar 2022, journal from conda does not support python3.9; since it is included during isce3 installation above, comment this out temporarily.
-lxml


Ok, we are removing it XD

It's still here - 3 lines above

vbrancat · 2023-02-28T19:26:05Z

requirements.txt

 scipy
 yamale
+h5py
+shapely


I though this dependency comes from isce3, do we need to have it in here?

When trying to build an environment from scratch, I recall needing to add these. I will try again and see if we can do without.

If isce3 is installed, COMPASS can run without these items. I will pare down list accordingly.

For h5py and shapely- aren't those things that we use specifically in here? It seems like we wouldn't want to rely on a chain of dependencies. e.g. if isce3 got rid of shapely, but we're still using it here, we want to still have it as a requirement.

Good point ☝️ Regardless it's innocuous to have these items in requirements.txt right?

vbrancat · 2023-02-28T19:46:10Z

tests/conftest.py

+        os.makedirs(dst_dir, exist_ok=True)
+
+    # download data
+    dataset_url = 'https://zenodo.org/record/7668411/files/'


Wondering if we should have a file inside the repository having all hard-coded paths. In this way, we do not need to hunt them when they change

So combine this with this? Perhaps into a yaml?

dataset_url: https://zenodo.org/record/7668411/files/ test_files: orbit_path: orbits/S1A_OPER_AUX_POEORB_OPOD_20221105T083813_V20221015T225942_20221017T005942.EOF' safe_path: S1A_IW_SLC__1SDV_20221016T015043_20221016T015111_045461_056FC0_6681.zip ...

vbrancat · 2023-02-28T19:50:45Z

tests/test_s1_geocode_slc.py

+    return np.s_[i_max_y:i_min_y, i_min_x:i_max_x]
+
+
+def test_geocode_slc_validate(geocode_slc_params):


I would prefer to have 2 separate tests. 1) testing that the actual workflow runs 2) validate the geolocation after running. Can we separate those two tests into separate functions?

vbrancat · 2023-02-28T19:59:20Z

tests/test_s1_geocode_slc.py

+        SimpleNamespace containing geocode SLC unit test parameters
+    '''
+    # get slice where corner reflectors should be
+    s_ = _get_reflectors_bounding_slice(geocode_slc_params)


The validation test I had in mind was a bit different and more in line with the validation activities that @seongsujeong is doing. We have geocoded a SLC, we do have the CR location from file, we can check that our geolocation is not messed up.

To do so:

based on the CR file, we can identify the CR lat/lons and extract the CR peak from the geocoded SLC

We can oversample the peak and get the CR location

We can check that the difference between the processed CR location and the one we get from file in X- and Y-direction and see whether we meet our requirement

Was this the plan?

What you described was the plan but I wasn't able to find all them. This is just a quick check to see any CRs were found so the unit tests would basic validation.

vbrancat · 2023-02-28T20:00:32Z

tests/test_s1_geocode_slc.py

+        arr = h5_obj[geocode_slc_params.raster_path][s_]
+
+    # check for bright spots in sliced array
+    corner_reflector_threshold = 3e3


How have you determined this threshold? Also, I do not fully understand the test. We are just testing for the presence of bright spots inside the processed CSLC?

Correct - this test only check for presence of bright spots in the processed CSLC within the area of the CRs. I was thinking to improve this to checking for individual CRs when we get all our corrections implemented.

I determined the threshold by plotting where the CRs are and picked a seemingly appropriate value based on the colorbar.

vbrancat

LGTM. Thanks for addressing my comments @LiangJYu :)

LiangJYu added 2 commits August 8, 2022 10:55

initial commit for geocode SLC unit test

6b02edd

download test data from zenodo

fd880ed

LiangJYu changed the title ~~WIP geocode SLC unit test~~ geocode SLC unit test Aug 22, 2022

LiangJYu marked this pull request as ready for review August 22, 2022 00:34

LiangJYu requested review from hfattahi, seongsujeong, vbrancat and yunjunz August 22, 2022 00:35

LiangJYu mentioned this pull request Aug 25, 2022

[New Feature]: persistent test data on CircleCI #51

Open

vbrancat reviewed Sep 12, 2022

View reviewed changes

tests/test_s1_geocode_slc.py Outdated Show resolved Hide resolved

tests/test_s1_geocode_slc.py Outdated Show resolved Hide resolved

tests/conftest.py Outdated Show resolved Hide resolved

tests/conftest.py Show resolved Hide resolved

added docstrings, more descriptive variable names, internet check

9f0b607

LiangJYu added 3 commits October 20, 2022 16:09

Merge branch 'main' into geocode_slc_unit_test

97c4461

run integration test in circleci

2984d49

adding pytest as requirement

1abbfe4

LiangJYu changed the title ~~geocode SLC unit test~~ geocode SLC integration test Oct 28, 2022

LiangJYu added 3 commits October 28, 2022 15:10

adding pytest to docker specfile

92daa0f

another attempt to run pytest in ci

0164652

yet another attempt to run pytest in ci

ebfea34

rtburns-jpl reviewed Nov 1, 2022

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

LiangJYu added 4 commits November 2, 2022 16:19

applying suggestion from rtburns

13cb5b2

docker command tweak

7a5655c

fix entry point

6442402

remove entry point

cd70f7c

rtburns-jpl reviewed Nov 8, 2022

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

LiangJYu added 2 commits November 8, 2022 12:25

fix path

6e3069c

activate environment

8eff671

seongsujeong reviewed Nov 18, 2022

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

docker/Dockerfile Outdated Show resolved Hide resolved

seongsujeong mentioned this pull request Nov 23, 2022

Docker for beta delivery #69

Merged

2 tasks

LiangJYu added 6 commits December 2, 2022 17:46

Merge remote-tracking branch 'upstream/main' into geocode_slc_unit_test

35cb9e8

updated specfile

5b4dc91

account for multiburst, new burst ID, geocode entire burst

07c83e4

use main instead of tag because of missing features

b30f823

clone hhtps not ssh

18c9cb4

template yaml

4ddf5f8

scottstanie mentioned this pull request Jan 4, 2023

WIP: Burst id unit tests isce-framework/s1-reader#84

Open

LiangJYu added 6 commits February 16, 2023 18:33

Merge branch 'main' into geocode_slc_unit_test

aa1540c

update unit tests for smaller dataset

8e10100

parallel test data download

88f0487

update test data URL update validation to check for presense of reflectors

update specifile

d438b58

fix test file paths update test data URL

remove unused imports

dd2e841

more comments/docstrings

e693f51

add environment yaml to help create clean/minimal environment add bounds check to margin application

LiangJYu requested review from rtburns-jpl, scottstanie, seongsujeong and vbrancat February 23, 2023 16:35

scottstanie approved these changes Feb 25, 2023

View reviewed changes

LiangJYu and others added 4 commits February 25, 2023 14:31

cleaner mp pool

aeb64ac

Co-authored-by: Scott Staniewicz <[email protected]>

fix docstring

3f72ad3

Co-authored-by: Scott Staniewicz <[email protected]>

load just the slice not EVERYTHING

b95f45c

Merge branch 'geocode_slc_unit_test' of github.com:LiangJYu/COMPASS i…

8953342

…nto geocode_slc_unit_test

vbrancat reviewed Feb 28, 2023

View reviewed changes

LiangJYu added 2 commits March 1, 2023 17:36

clarity in file related variable

dd7b5e1

fix typo in path

fa33f64

vbrancat approved these changes Mar 6, 2023

View reviewed changes

LiangJYu merged commit 8e0545e into opera-adt:main Mar 6, 2023

		return np.s_[i_max_y:i_min_y, i_min_x:i_max_x]


		def test_geocode_slc_validate(geocode_slc_params):

		corner_reflector_threshold = 3e3
		assert np.any(arr > corner_reflector_threshold)

		tests/product
		tests/scratch

geocode SLC integration test #46

geocode SLC integration test #46

Uh oh!

Conversation

LiangJYu commented Aug 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vbrancat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LiangJYu commented Sep 12, 2022

Uh oh!

LiangJYu commented Sep 14, 2022

Uh oh!

rtburns-jpl commented Sep 14, 2022

Uh oh!

rtburns-jpl commented Nov 1, 2022

Uh oh!

Uh oh!

Uh oh!

seongsujeong left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

LiangJYu commented Nov 19, 2022

Uh oh!

scottstanie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scottstanie Mar 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbrancat left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

LiangJYu commented Aug 9, 2022 •

edited

Loading

seongsujeong left a comment •

edited

Loading

scottstanie Mar 2, 2023 •

edited

Loading

LiangJYu Mar 2, 2023 •

edited

Loading

LiangJYu Mar 2, 2023 •

edited

Loading