Skip to content

compatibility with gsl 2.7 #32607

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

Closed
yyyyx4 opened this issue Oct 2, 2021 · 18 comments
Closed

compatibility with gsl 2.7 #32607

yyyyx4 opened this issue Oct 2, 2021 · 18 comments

Comments

@yyyyx4
Copy link
Member

yyyyx4 commented Oct 2, 2021

GSL 2.7 changes the definition of gsl_complex; see https://groups.google.com/g/sage-devel/c/yQ67Wy0gp58/m/jr6f7xtcBQAJ and #32587.

This patch changes the library code to use the macros provided by GSL instead of directly accessing the data structure, which should make things work with GSL 2.7 as well as older versions.

Depends on #32659

CC: @mkoeppe

Component: interfaces

Keywords: gsl

Author: Lorenz Panny

Branch/Commit: 6633bc4

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/32607

@yyyyx4 yyyyx4 added this to the sage-9.5 milestone Oct 2, 2021
@yyyyx4
Copy link
Member Author

yyyyx4 commented Oct 2, 2021

Commit: 6633bc4

@yyyyx4
Copy link
Member Author

yyyyx4 commented Oct 2, 2021

New commits:

6633bc4use GSL macros for compatibility with gsl 2.7

@yyyyx4
Copy link
Member Author

yyyyx4 commented Oct 2, 2021

Branch: public/32607

@yyyyx4
Copy link
Member Author

yyyyx4 commented Oct 2, 2021

Author: Lorenz Panny

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 2, 2021

comment:2

multiple errors in the patchbot run

@yyyyx4
Copy link
Member Author

yyyyx4 commented Oct 2, 2021

comment:3

The bot seems to be running old versions of some these files. In particular, the most common error is

      File "sage/ext/interpreters/wrapper_cdf.pyx", line 17, in sage.ext.interpreters.wrapper_cdf.dz_to_CDE (build/cythonized/sage/ext/interpreters/wrapper_cdf.c:3144)
        z._complex.real = creal(dz)
    AttributeError: 'dict' object has no attribute 'real'

— but line 17 of wrapper_cdf.pyx was changed in this patch (indirectly via sage_setup/autogen/interpreters/specs/cdf.py) to something different from what the error message shows.

I suppose the bot should be instructed to regenerate wrapper_cdf.pyx somehow?

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 2, 2021

comment:4

Then the patchbot won't be helpful after all. Next step: Portability testing on GH Actions

@yyyyx4
Copy link
Member Author

yyyyx4 commented Oct 7, 2021

comment:5

https://github.com/kgywlytnch/sagetrac-mirror/actions/runs/1301872704

Did I do this right? The failures seem unrelated to what we're doing here.

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 7, 2021

comment:6

Yes, looking good. In particular works without failure on ubuntu-bionic-standard, where it's using system GSL 2.4+dfsg-6

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 7, 2021

Reviewer: Matthias Koeppe

@vbraun
Copy link
Member

vbraun commented Oct 9, 2021

comment:9
sage: complex_plot(sqrt(x), (-5, 5), (-5, 5))
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
AttributeError: 'dict' object has no attribute 'real'
Exception ignored in: 'sage.ext.interpreters.wrapper_cdf.CDE_to_dz'
Traceback (most recent call last):
  File "/home/release/Sage/local/lib64/python3.9/site-packages/sage/misc/decorators.py", line 491, in wrapper
    return func(*args, **options)
AttributeError: 'dict' object has no attribute 'real'
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-1-248579947237> in <module>
----> 1 complex_plot(sqrt(x), (-Integer(5), Integer(5)), (-Integer(5), Integer(5)))

~/Sage/local/lib64/python3.9/site-packages/sage/misc/lazy_import.pyx in sage.misc.lazy_import.LazyImport.__call__ (build/cythonized/sage/misc/lazy_import.c:4052)()
    360             True
    361         """
--> 362         return self.get_object()(*args, **kwds)
    363 
    364     def __repr__(self):

~/Sage/local/lib64/python3.9/site-packages/sage/misc/decorators.py in wrapper(*args, **kwds)
    489                 options['__original_opts'] = kwds
    490             options.update(kwds)
--> 491             return func(*args, **options)
    492 
    493         #Add the options specified by @options to the signature of the wrapped

~/Sage/local/lib64/python3.9/site-packages/sage/plot/complex_plot.pyx in sage.plot.complex_plot.complex_plot (build/cythonized/sage/plot/complex_plot.c:5949)()
    398         for x in srange(*x_range, include_endpoint=True):
    399             sig_check()
--> 400             row.append(f(new_CDF_element(x, y)))
    401         z_values.append(row)
    402 

~/Sage/local/lib64/python3.9/site-packages/sage/ext/interpreters/wrapper_cdf.pyx in sage.ext.interpreters.wrapper_cdf.Wrapper_cdf.__call__ (build/cythonized/sage/ext/interpreters/wrapper_cdf.c:4068)()
    108         for i from 0 <= i < len(args):
    109             self._args[i] = CDE_to_dz(args[i])
--> 110         return dz_to_CDE(interp_cdf(c_args
    111             , self._constants
    112             , self._py_constants

~/Sage/local/lib64/python3.9/site-packages/sage/ext/interpreters/wrapper_cdf.pyx in sage.ext.interpreters.wrapper_cdf.dz_to_CDE (build/cythonized/sage/ext/interpreters/wrapper_cdf.c:3142)()
     15 cdef inline ComplexDoubleElement dz_to_CDE(double_complex dz):
     16     cdef ComplexDoubleElement z = <ComplexDoubleElement>ComplexDoubleElement.__new__(ComplexDoubleElement)
---> 17     z._complex.real = creal(dz)
     18     z._complex.imag = cimag(dz)
     19     return z

AttributeError: 'dict' object has no attribute 'real'

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 9, 2021

comment:10

... that's the issue with incremental builds, same as comment:3

@vbraun
Copy link
Member

vbraun commented Oct 9, 2021

comment:11

hmm well that sounds like a problem with the dependencies, is it fixable?

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 9, 2021

comment:12

The relevant code is in src/sage_setup/autogen/interpreters/__init__.py, I haven't looked at the details

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 9, 2021

comment:13

I am working on a fix.

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 10, 2021

comment:14

Fixed in #32659

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 10, 2021

Dependencies: #32659

@vbraun
Copy link
Member

vbraun commented Oct 20, 2021

Changed branch from public/32607 to 6633bc4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants