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

Incompatibility with python3.10 #458

Closed
sssangha opened this issue Mar 15, 2022 · 26 comments · Fixed by #534
Closed

Incompatibility with python3.10 #458

sssangha opened this issue Mar 15, 2022 · 26 comments · Fixed by #534
Labels
bug Something isn't working

Comments

@sssangha
Copy link
Contributor

In the rangecoreg step, topsApp crashes with the following error message:

....
  File "/u/trappist-r0/ssangha/conda_installation/stable_mach14_2022/envs/ARIA-tools/lib/python3.10/site-packages/isce/components/iscesys/C
omponent/Component.py", line 179, in __call__
    return getattr(self, self.__class__.__name__.lower())(*args)
  File "/u/trappist-r0/ssangha/conda_installation/stable_mach14_2022/envs/ARIA-tools/lib/python3.10/site-packages/isce/components/mroipac/a
mpcor/Ampcor.py", line 335, in ampcor
    self.setState()
  File "/u/trappist-r0/ssangha/conda_installation/stable_mach14_2022/envs/ARIA-tools/lib/python3.10/site-packages/isce/components/mroipac/a
mpcor/Ampcor.py", line 484, in setState
    ampcor.setImageDataType1_Py(str(self.imageDataType1))
SystemError: PY_SSIZE_T_CLEAN macro must be defined for '#' formats

As documented in other cases (https://stackoverflow.com/questions/70705404/systemerror-py-ssize-t-clean-macro-must-be-defined-for-formats), this error results from isce not being compatible with python3.10.

However, I've confirmed this error is circumvented with using an earlier version (python 3.9.10). So until this compatibility issue is resolved, <python3.10 dependencies should be enforced.

sssangha added a commit to sssangha/isce2 that referenced this issue Mar 15, 2022
See isce-framework#458 for detailed description of issue addressed here in the Readme
sssangha added a commit to sssangha/isce2 that referenced this issue Mar 15, 2022
See isce-framework#458 for detailed description of issue addressed here in the Readme
@sssangha
Copy link
Contributor Author

While I suggested a change to the readme in PR #459 , we need to also impose a hard constraint for now in the conda distribution, with which I experienced these issues

@rtburns-jpl
Copy link
Member

I pushed a commit to #424 to try to fix this, can you try again on that branch and see if it works now?

@yunjunz
Copy link
Member

yunjunz commented May 17, 2022

I got the similar SystemError: PY_SSIZE_T_CLEAN as below while running dem.py using conda installed isce2 in python 3.10.

dem.py -a stitch -b 37 40 -119 -114 -r -s 1 -c
...
API open (WR): demLat_N37_N40_Lon_W119_W114.dem.wgs84
Traceback (most recent call last):
  File "/home/zyunjun/tools/miniconda3/envs/insar/lib/python3.10/site-packages/isce/applications/dem.py", line 171, in <module>
    sys.exit(main())
  File "/home/zyunjun/tools/miniconda3/envs/insar/lib/python3.10/site-packages/isce/applications/dem.py", line 141, in main
    demImg = ds.correct()
  File "/home/zyunjun/tools/miniconda3/envs/insar/lib/python3.10/site-packages/isce/components/contrib/demUtils/DemStitcher.py", line 862, in correct
    return cg(image,conversionType) if image else cg(self._image,conversionType)
  File "/home/zyunjun/tools/miniconda3/envs/insar/lib/python3.10/site-packages/isce/components/contrib/demUtils/Correct_geoid_i2_srtm.py", line 182, in __call__
    self.correct_geoid_i2_srtm()
  File "/home/zyunjun/tools/miniconda3/envs/insar/lib/python3.10/site-packages/isce/components/contrib/demUtils/Correct_geoid_i2_srtm.py", line 196, in correct_geoid_i2_srtm
    self.setState()
  File "/home/zyunjun/tools/miniconda3/envs/insar/lib/python3.10/site-packages/isce/components/contrib/demUtils/Correct_geoid_i2_srtm.py", line 239, in setState
    correct_geoid_i2_srtm.setGeoidFilename_Py(self.geoidFilename)
SystemError: PY_SSIZE_T_CLEAN macro must be defined for '#' formats

I tried to add #define PY_SSIZE_T_CLEAN to correct_geoid_i2_srtmmodule.cpp#L30, then the following Segmentation fault error came out instead:

API open (R): ./demLat_N37_N40_Lon_W119_W114.dem
API close:  ./demLat_N37_N40_Lon_W119_W114.dem
Writing geotrans to VRT for ./demLat_N37_N40_Lon_W119_W114.dem
GDAL open (R): ./demLat_N37_N40_Lon_W119_W114.dem.vrt
API open (WR): demLat_N37_N40_Lon_W119_W114.dem.wgs84
Segmentation fault

@EJFielding
Copy link
Contributor

I hit this problem with a Python 3.10 installed by Anaconda also. Is there a way to define the required PY_SSIZE_T_CLEAN globally in all of ISCE2 to make it compatible with Python 3.10? Or does it require other changes?

@rtburns-jpl
Copy link
Member

If you're using cmake, you can try export CFLAGS=-DPY_SSIZE_T_CLEAN and export CXXFLAGS=-DPY_SSIZE_T_CLEAN before running cmake configure, that should set those macros globally for all C/C++ code. Is that sufficient to make it compatible?

@EJFielding
Copy link
Contributor

I ran into this problem in the NISAR On-demand system where the environment setup notebook installs ISCE2 (and ISCE3) with Anaconda and a yml file that lists isce2 and all the dependencies. Anaconda defaults to Python 3.10 now.

@rtburns-jpl
Copy link
Member

@yunjunz I think I figured out why you're getting a segfault here. Python3.10 changed the size type, so any time we use PyArg_ParseTuple with a 's#' argument we need to change the result var from an int to a Py_ssize_t. Can you check if defining PY_SSIZE_T_CLEAN along with this patch this fixes your dem.py workflow?

--- a/contrib/demUtils/correct_geoid_i2_srtm/bindings/correct_geoid_i2_srtmmodule.cpp
+++ b/contrib/demUtils/correct_geoid_i2_srtm/bindings/correct_geoid_i2_srtmmodule.cpp
@@ -172,12 +172,13 @@ PyObject * setConversionType_C(PyObject* self, PyObject* args)
 PyObject * setGeoidFilename_C(PyObject* self, PyObject* args)
 {
     char * varChar;
-    int  var;
+    Py_ssize_t var;
     if(!PyArg_ParseTuple(args, "s#", &varChar ,&var))
     {
         return NULL;
     }
-    setGeoidFilename_f(varChar,&var);
+    int ivar = var;
+    setGeoidFilename_f(varChar, &ivar);
     return Py_BuildValue("i", 0);
 }

I'll try to fix the other occurrences of this in isce2 to hopefully get us working with python 3.10.

@yunjunz
Copy link
Member

yunjunz commented Jul 21, 2022

Thank you for fixing @rtburns-jpl. I was trying to test it, but there is a problem while downloading SRTM from the USGS website, so I could not confirm if the fix works or not.

@yuankailiu
Copy link
Contributor

Thank you for fixing @rtburns-jpl. I was trying to test it, but there is a problem while downloading SRTM from the USGS website, so I could not confirm if the fix works or not.

Yeh, @yunjunz, right now, it shows an URL error for me

curl -n  -L -c $HOME/.earthdatacookie -b $HOME/.earthdatacookie -k -f -O http://e4ftl01.cr.usgs.gov/MEASURES/SRTMGL1.003/2000.02.11/N4
1W125.SRTMGL1.hgt.zip
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0   299    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
curl: (22) The requested URL returned error: 503
2022-07-21 12:48:57,859 - isce.contrib.demUtils.DemStitcher - WARNING - There was a problem in retrieving the file  http://e4ftl01.cr.
usgs.gov/MEASURES/SRTMGL1.003/2000.02.11/N41W125.SRTMGL1.hgt.zip. Exception

@rtburns-jpl
Copy link
Member

That's unfortunate. I thought this was a temporary outage but maybe their download url format has changed? Should we open a separate issue for that?

@EJFielding
Copy link
Contributor

If they changed the SRTM download URLs, then I would recommend a new issue. I was going to open one earlier this week but then it worked for me again at least once. I did not find any announcements about changes but they tend to change it about every two years.

@AhmetMA
Copy link

AhmetMA commented Jul 24, 2022

If they changed the SRTM download URLs, then I would recommend a new issue. I was going to open one earlier this week but then it worked for me again at least once. I did not find any announcements about changes but they tend to change it about every two years.

A recently updated thread regarding the connectivity issues:
https://forum.earthdata.nasa.gov/viewtopic.php?t=2262#p11891

@sisi-ali
Copy link

sisi-ali commented Aug 4, 2022

I have had this error for about a month now.
Has anyone encountered this error before and solved it

18ea23c6-aff8-4a23-a556-0cc8537178c8

@EJFielding
Copy link
Contributor

I have had this error for about a month now. Has anyone encountered this error before and solved it

Hi sisi-ali,

Yes, we are all getting similar errors. It seems the Land Processes DAAC servers are overloaded for a long time. It works occasionally.

@sisi-ali
Copy link

sisi-ali commented Aug 5, 2022

thank for reply me
But I can't download even though I'm always trying.
You have no solution for this problem. I really need this problem to be solved so that I can move forward with my work

@yunjunz
Copy link
Member

yunjunz commented Aug 10, 2022

@yunjunz I think I figured out why you're getting a segfault here. Python3.10 changed the size type, so any time we use PyArg_ParseTuple with a 's#' argument we need to change the result var from an int to a Py_ssize_t. Can you check if defining PY_SSIZE_T_CLEAN along with this patch this fixes your dem.py workflow?

--- a/contrib/demUtils/correct_geoid_i2_srtm/bindings/correct_geoid_i2_srtmmodule.cpp
+++ b/contrib/demUtils/correct_geoid_i2_srtm/bindings/correct_geoid_i2_srtmmodule.cpp
@@ -172,12 +172,13 @@ PyObject * setConversionType_C(PyObject* self, PyObject* args)
 PyObject * setGeoidFilename_C(PyObject* self, PyObject* args)
 {
     char * varChar;
-    int  var;
+    Py_ssize_t var;
     if(!PyArg_ParseTuple(args, "s#", &varChar ,&var))
     {
         return NULL;
     }
-    setGeoidFilename_f(varChar,&var);
+    int ivar = var;
+    setGeoidFilename_f(varChar, &ivar);
     return Py_BuildValue("i", 0);
 }

I'll try to fix the other occurrences of this in isce2 to hopefully get us working with python 3.10.

@rtburns-jpl with #560 being merged, I confirmed dem.py works fine under python 3.10 with the changes you laid out. Thank you and cheers!

@EJFielding
Copy link
Contributor

@sisi-ali Please get the new version of ISCE2. We have finally found the necessary change to the DEM downloading code in #560.

@sisi-ali
Copy link

Hi everyone
I want to calculate the closure_phase_bias.py using the MintPy software that I per-processed on ISCE,
but I cant calculate with (https://github.com/insarlab/MintPy/blob/main/mintpy/closure_phase_bias.py)
I would be grateful if you could guide me

@EJFielding
Copy link
Contributor

@sisi-ali It would be better to ask on the MintPy discussion site https://github.com/insarlab/MintPy/discussions or on their Google Group.

@sisi-ali
Copy link

sisi-ali commented Oct 11, 2022 via email

@krive050
Copy link

Hello,

I running ISCE 2.6.1. The topsApp.py crashes at the DEM steps when it is trying to convert to DEM.

I am getting the same error "SystemError: PY_SSIZE_T_CLEAN macro must be defined for '#' formats"

Is anyone able to help me with this? Thanks!

@EJFielding
Copy link
Contributor

@krive050 The easy workaround is to change your Python to version 3.9 instead of version 3.10.

@rtburns-jpl
Copy link
Member

@krive050 Please check if #534 resolves your issue. I will merge that PR once I have verification that any workflow is fixed by it.

@rtburns-jpl rtburns-jpl added the bug Something isn't working label Nov 1, 2022
@JolinnaYoung
Copy link

@yunjunz Hi Dr.Yunjun! I met the same question when using dem.py with Python's version = 3.10. I've tried to add #define PY_SSIZE_T_CLEAN to [correct_geoid_i2_srtmmodule.cpp#L30] but nothing changed at all. Could you please give me hints on solving this problem? Many thanks!

@EJFielding
Copy link
Contributor

@yunjunz Hi Dr.Yunjun! I met the same question when using dem.py with Python's version = 3.10. I've tried to add #define PY_SSIZE_T_CLEAN to [correct_geoid_i2_srtmmodule.cpp#L30] but nothing changed at all. Could you please give me hints on solving this problem? Many thanks!

Hi Jolinna, you should upgrade to ISCE2 version 2.6.3. It has been updated to work with Python 3.10.

@JolinnaYoung
Copy link

@yunjunz Hi Dr.Yunjun! I met the same question when using dem.py with Python's version = 3.10. I've tried to add #define PY_SSIZE_T_CLEAN to [correct_geoid_i2_srtmmodule.cpp#L30] but nothing changed at all. Could you please give me hints on solving this problem? Many thanks!

Hi Jolinna, you should upgrade to ISCE2 version 2.6.3. It has been updated to work with Python 3.10.

Hi Eric, thanks for your kind reply! I'll try it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants