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

dialect::dimensions object's contents not accessible from python swig wrapper #50

Open
shakfu opened this issue May 21, 2021 · 13 comments

Comments

@shakfu
Copy link

shakfu commented May 21, 2021

Hi,

The python swig wrapper works great except that I am unable to access the contents of the dimensions objects which is a std::pair<double, double> instance return from DialectNode.getDimensions.

dialect::dimensions is a typedef std::pair<double,double> dimensions

But I am not able to retrieve its contents in python.

In [1]: from adaptagrams import *

In [2]: n = DialectNode.allocate(10.0, 20.0, 30.0, 30.0)

In [3]: d = n.getDimensions()

In [4]: d.first
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-4-6ed88d6b1bba> in <module>
----> 1 d.first

AttributeError: 'SwigPyObject' object has no attribute 'first'

I have tried to add some additional wrapping code to adaptagrams.i. The following gives the same error as above:

%typedef std::pair<double,double> dimensions;
%template() std::pair<double,double>;
%rename(dimensions) dialect::dimensions;

and when I add a %template(dimensions) std::pair<double,double>; it generates the wrapping code but then cannot find it in the resultant extension module.

My knowledge of SWIG is quite limited so would appreciate some advice on how to fix this.

@shakfu
Copy link
Author

shakfu commented May 22, 2021

FYI, I have managed to build an example python3 extension of code with the same signature as dialect::dimensions. I have attached a zip file of two cases: one with and one without the dialect namespace.
test_dimensions.zip

There seems to be no need to use %rename as per my earlier assumption.

@shakfu
Copy link
Author

shakfu commented May 22, 2021

Still stuck, If I insert:

%template() std::pair <double,double>;
%template(dimensions) std::pair <double,double>;

into adaptagrams.i, I still get

AttributeError: module '_adaptagrams' has no attribute 'dimensions_first_get'

when I import from anything from adaptagrams.py post-compilation.

@skieffer
Copy link
Collaborator

Hi, unfortunately it's been ages since I did anything with SWIG, and it will be a long time before I could give this any serious attention. Hopefully someone else can help. If I do think of anything I will chime in. Good luck!

@shakfu
Copy link
Author

shakfu commented May 23, 2021

Thanks for your reply @skieffer.

It's a really strange issue, since the isolated case seems to work as expected. it may be the case that the order of one of the other declarations is interfering with the expected outcome. Swig isn't being too helpful about this... let's see..

In any case, it's not the end of the world if it isn't resolved since one can read/write from the .tglf format.

@shakfu
Copy link
Author

shakfu commented May 25, 2021

I have asked for advice on this issue over at stackoverflow

@jtiai
Copy link

jtiai commented May 25, 2021 via email

@shakfu
Copy link
Author

shakfu commented May 25, 2021

@jtiai I'm just a user of the library. (-:

I think the benefit of SWIG is the automated wrapping, Cython, pybind11 and cffi are manual wrapping tools. I've personally only had experience with cython previously but I hear pybind11 is a good option for c++

@jtiai
Copy link

jtiai commented May 25, 2021

I actually have wrapped part of libavoid with pybind11. Wasn't much of a work.

@shakfu
Copy link
Author

shakfu commented May 25, 2021

@jtiai the missing part of the swig interface in this case is really trivial and tiny: just a std::pair<double,double> and it's just ridiculously weird that swig will generate the code but won't enable it. I've moved the declaration all over the adaptagrams.i file without success in case it was an order issue.

Now I'm just irritated about not finding a solution, and having exhausted my own attempts, so I've just posted it on the swig forum in case there's an expert out there who can shed light on this problem: https://sourceforge.net/p/swig/mailman/message/37289720/

S

@shakfu
Copy link
Author

shakfu commented May 31, 2021

I took @jtiai's advice and wrapped up the part I wanted (HOLA in libdialect) using pybind11

@skieffer In case someone else finds this useful, I put the work in a GitHub repo. The pybind11 wrapper for HOLA is in a folder named pybind_adaptagrams

S

@shakfu
Copy link
Author

shakfu commented Jul 26, 2023

@skieffer @jtiai

As a followup to the above: there is now a project repo for the pybind11 wrapper: pyhola

In practice it seems to perform somewhat consistently with the swig wrapper and holds its own against other algorithms.

@skieffer
Copy link
Collaborator

skieffer commented Aug 7, 2023

@shakfu, thanks for your work with pyhola! Difficulties often seem to arise with SWIG, so having an alternative integration with Python is great.

Your layout comparison is interesting. I wonder why the adaptagrams and pyhola layouts are different. Different versions of adaptagrams? Different starting positions?

Also the network seems to be (almost?) a tree. Is the "speaker" box connected twice to its neighbor? If so, are ports being used, to model a multigraph? It would be interesting to see how it performed with a network that had more non-tree structure.

@shakfu
Copy link
Author

shakfu commented Aug 7, 2023

Hi @skieffer

Thanks for your reply.

Your layout comparison is interesting. I wonder why the adaptagrams and pyhola layouts are different. Different versions of adaptagrams? Different starting positions?

The underlying graph is the same but the differences in layout may be because the adaptagrams libs I use to build pyhola are relatively old compared to the current adaptagrams repo state.

This motivated me to add a basic cmake build system which you have commented on in another post.

Also the network seems to be (almost?) a tree. Is the "speaker" box connected twice to its neighbor? If so, are ports being used, to model a multigraph? It would be interesting to see how it performed with a network that had more non-tree structure.

In Max/MSP objects have inlets and outlets. The speaker models a DAC so the two cables from one object's outlet represent a mono cable connecting to left and right inlets of the DAC (which are the left and right audio outputs of the system).

Note that the layout comparison compares only node position layout, edges are not yet considered. I would like progress in that direction but my initial pybind11 wrapper was a bit limited. This pushed me to try to wrap all of the adaptagram libs via an automatic binder generation tool.

This is working surprisingly well, but as before, it makes no sense to do so with old adaptagrams libs... and hence more justification for the cmake project..

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