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

Octave support #74

Open
kupiqu opened this issue Apr 17, 2019 · 28 comments
Open

Octave support #74

kupiqu opened this issue Apr 17, 2019 · 28 comments

Comments

@kupiqu
Copy link

kupiqu commented Apr 17, 2019

wondering whether it would be hard to support octave, given that oct2py does not seem to be developed anymore (last release was ~2 years ago)

@bastibe
Copy link
Owner

bastibe commented Apr 19, 2019

Most of the core interaction in transplant should work without issue in Octave. The problem is the communication channel. Transplant uses zeromq as its socket implementation, and calls into libzmq using loadlibrary in Matlab. Octave does not support loadlibrary. The interface wouldn't be hard to implement, though, probably as an oct-file.

If you'd like to take a stab at this and re-implement ZMQ.m for Octave, I'd be grateful for a pull request! As long as the signature of ZMQ.m is not changed, I suspect the rest of Transplant would work with few modifications.

But I am not a user of Octave, and probably won't find the time to do it myself.

@kupiqu
Copy link
Author

kupiqu commented Apr 19, 2019

I'll check it out. Thank you

@kupiqu
Copy link
Author

kupiqu commented Apr 20, 2019

I checked and it seemed that the main issue against implementing loadlibrary in Octave is not there anymore, so I opened a Matlab compatibility issue in Octave to see if an official implementation of the function could be possible (https://savannah.gnu.org/bugs/index.php?56176).

Are there other things that need to be adapted in addition to loadlibrary?

@apjanke
Copy link

apjanke commented Apr 20, 2019

loadlibrary is a significant low-level function, so it's unlikely to make it into a released Octave any time soon.

But there's an Octave Forge zeromq package that provides ZeroMQ bindings for Octave using the oct-file linkage mechanism. Maybe you could write a ZMQ.m for Octave that uses that?

@apjanke
Copy link

apjanke commented Apr 20, 2019

(BTW, Transplant looks like a cool library. I've added it to my public list of interesting FLOSS Matlab projects.)

@bastibe
Copy link
Owner

bastibe commented Apr 20, 2019

Oh, cool! The zeromq package on Octave Forge should do the trick! It would probably be easiest to make ZMQ.m Octave-aware, and have a special code path in Octave that uses the zeromq package.

@kupiqu
Copy link
Author

kupiqu commented Apr 20, 2019

I installed the zeromq package.

It would probably be easiest to make ZMQ.m Octave-aware

Sounds like the way to go... I never used zeromq before, but will try...

have a special code path in Octave that uses the zeromq package

what do you mean by special code path in Octave? (sorry if this is obvious)

@bastibe
Copy link
Owner

bastibe commented Apr 21, 2019

My idea is to have all Octave-specific code confined to ZMQ.m if possible. So we would have to modify ZMQ.m to have two code-paths, one using loadlibrary for Matlab, and one using the zeromq package for Octave. Both code paths (essentially ifs) should be able to co-exist in the same file.

Would you like to take a stab at this? I'd be really grateful for a pull request!

@RobBW
Copy link

RobBW commented Apr 21, 2019 via email

@kupiqu
Copy link
Author

kupiqu commented Apr 21, 2019

Would you like to take a stab at this? I'd be really grateful for a pull request!

Sure

@kupiqu
Copy link
Author

kupiqu commented Apr 21, 2019

I started with it, but found out that zeromq package does not support zmq_ctx_new and zmq_ctx_term. I think it may be ok, but wanted to check it out, as I am a zmq newbie :)

Beyond that, there are issues in the arguments to pass to Matlab vs. Octave. Octave does not support -nojvm and -nodesktop (there is --no-window-system and --no-gui, though), and -r functionality is accomplished with --eval.

I have a way to distinguish matlab from octave in m-code (in the context of ZMQ.m), but we will need of sth similar in python (in the context of transplant_master.py). What do you think would be a good way to add this?I guess we can add a transplant input argument to specify matlab vs. octave...

@kupiqu
Copy link
Author

kupiqu commented Apr 21, 2019

Mmm, I'm stacked with this:

                msg = libstruct('zmq_msg_t', struct('hidden', zeros(1, 64, 'uint8')));
                calllib('libzmq', 'zmq_msg_init', msg); % always returns 0
                msglen = calllib('libzmq', 'zmq_msg_recv', msg, obj.socket, 0);
                assert(msglen >= 0, obj.errortext('zmq_msg_recv'));
                msgptr = calllib('libzmq', 'zmq_msg_data', msg);
                if not(msgptr.isNull)
                    setdatatype(msgptr, 'uint8Ptr', 1, msglen);
                    str = uint8(msgptr.Value);
                else
                    str = uint8([]);
                end
                err = calllib('libzmq', 'zmq_msg_close', msg);
                assert(err == 0, obj.errortext('zmq_msg_close'));

Octave's zero_mq does not support zmq_msg_* functions, only zmq_recv... :/

EDIT: not a problem, octave forge package simplifies all this complexity (context and msg functions are internal)

@bastibe
Copy link
Owner

bastibe commented Apr 23, 2019

I have a way to distinguish matlab from octave in m-code (in the context of ZMQ.m), but we will need of sth similar in python (in the context of transplant_master.py). What do you think would be a good way to add this?I guess we can add a transplant input argument to specify matlab vs. octave...

True, I hadn't thought of that. We'll have to start a different binary, too. I think the simplest way to do this would be a subclass of Matlab or TransplantMaster that calls octave instead of matlab, with the appropriate arguments.

@kupiqu
Copy link
Author

kupiqu commented Apr 23, 2019

True, I hadn't thought of that. We'll have to start a different binary, too. I think the simplest way to do this would be a subclass of Matlab or TransplantMaster that calls octave instead of matlab, with the appropriate arguments.

I implemented this the easy way, simply adding an argument called backend to the Matlab class, which defaults to "matlab" (vs. "octave"). And then a simple conditional sets the proper arguments. I hope it is ok. This is sth that can be improved later if you are not convinced about it.

@bastibe
Copy link
Owner

bastibe commented Apr 24, 2019

We'll have a look at the API once it actually works. For now, this is perfectly fine. (I am a strong believer in "tracer bullets": prototype implementations that solve the central problem, but none of the edge cases. Once a prototype is known to work, I like to iterate on the surrounding API. But I find it wasted effort to focus too much on the API while still working on the core algorithms; Too many things still change at that stage, to concern oneself with such details.)

@kupiqu
Copy link
Author

kupiqu commented Apr 25, 2019

I get the following errors in Octave in comparison to Matlab:

Matlab:

libzmq loaded
socket done
connection done
receiving msg:
{"name":"eval","type":"get_global"}
msg received.
sending msg:
{"type":"value","value":["__function__","eval"]}
msg sent.
receiving msg:
{"name":"eval","args":["0;"],"nargout":-1,"type":"call"}
msg received.
sending msg:
{"type":"value","value":0}

Octave:

zeromq loaded
socket done
connection done
receiving msg:
{"name":"eval","type":"get_global"}
msg received.
sending msg:
{"identifier":"Octave:undefined-function","message":"'idx' undefined near line 236 column 13","stack":[{"file":"\/home\/miniconda\/envs\/cenv1\/lib\/python3.7\/site-packages\/transplant\/parsejson.m","name":"parsejson>false","line":236,"column":11,"scope":null,"context":0},{"file":"\/home\/miniconda\/envs\/cenv1\/lib\/python3.7\/site-packages\/transplant\/parsejson.m","name":"parsejson>object","line":143,"column":9,"scope":null,"context":0},{"file":"\/home\/miniconda\/envs\/cenv1\/lib\/python3.7\/site-packages\/transplant\/parsejson.m","name":"parsejson>value","line":50,"column":20,"scope":null,"context":0},{"file":"\/home\/miniconda\/envs\/cenv1\/lib\/python3.7\/site-packages\/transplant\/parsejson.m","name":"parsejson","line":27,"column":16,"scope":null,"context":0},{"file":"\/home\/miniconda\/envs\/cenv1\/lib\/python3.7\/site-packages\/transplant\/transplant_remote.m","name":"transplant_remote>receive_msg","line":200,"column":21,"scope":null,"context":0},{"file":"\/home\/miniconda\/envs\/cenv1\/lib\/python3.7\/site-packages\/transplant\/transplant_remote.m","name":"transplant_remote","line":71,"column":17,"scope":null,"context":0}],"type":"error"}
msg sent.
receiving msg:
{"name":"what","type":"get_global"}
msg received.
sending msg:
{"type":"value","value":["__function__","what"]}
msg sent.
receiving msg:
{"name":"what","args":["eval"],"nargout":-1,"type":"call"}
msg received.
sending msg:
{"identifier":null,"message":"what: could not find the directory eval","stack":[{"file":"\/app\/share\/octave\/5.1.0\/m\/miscellaneous\/what.m","name":"what","line":85,"column":9,"scope":null,"context":0},{"file":"\/home\/miniconda\/envs\/cenv1\/lib\/python3.7\/site-packages\/transplant\/transplant_remote.m","name":"transplant_remote","line":129,"column":38,"scope":null,"context":0}],"type":"error"}
msg sent.
receiving msg:

And then Octave gets stuck forever in there....

There are errors while sending info coming from transplant_remote.m when calling the "what" and "idx". The first exists in Octave but there should be some sort of compatibility issue. Not sure what is going on with "idx".

This is getting trickier as the mcode does not seem octave compatible...

EDIT: I confused "message" with "idx", fixed.

@kupiqu
Copy link
Author

kupiqu commented Apr 25, 2019

I'm a bit lost with those errors, if you know how to deal with that, please let me know...

@kupiqu
Copy link
Author

kupiqu commented Apr 25, 2019

I guess the original issue is that Octave is not able to send this message:

{"type":"value","value":["__function__","eval"]}

@bastibe
Copy link
Owner

bastibe commented Apr 27, 2019

I think it first gets lost in parsejson.m. Can you try and see if parsejson works in octave at all?

@apjanke
Copy link

apjanke commented Apr 27, 2019

Wow, you have a whole JSON decoder in there!

You might not need that. Matlab R2016b and newer have jsonencode() and jsondecode() functions now. Octave doesn't but I've written a JSON package for it that provides Matlab-compatible jsonencode and jsonencode implementations: https://github.com/apjanke/octave-jsonstuff

@kupiqu
Copy link
Author

kupiqu commented Apr 28, 2019

I think it first gets lost in parsejson.m. Can you try and see if parsejson works in octave at all?

In addition to parsejson, it seems there is sth else. When using msgpack, the messages look the same, however Matlab properly ends communication whereas Octave gets stuck:

Matlab:

libzmq loaded
socket done
connection done
receiving msg:
  Columns 1 through 13

   130   164   110    97   109   101   164   101   118    97   108   164   116

  Columns 14 through 26

   121   112   101   170   103   101   116    95   103   108   111    98    97

  Column 27

   108

msg received.
sending msg:
  Columns 1 through 13

   130   164   116   121   112   101   165   118    97   108   117   101   165

  Columns 14 through 26

   118    97   108   117   101   146   172    95    95   102   117   110    99

  Columns 27 through 37

   116   105   111   110    95    95   164   101   118    97   108

msg sent.
receiving msg:
  Columns 1 through 13

   132   164   110    97   109   101   164   101   118    97   108   164    97

  Columns 14 through 26

   114   103   115   145   162    48    59   167   110    97   114   103   111

  Columns 27 through 39

   117   116   255   164   116   121   112   101   164    99    97   108   108

msg received.
sending msg:
  Columns 1 through 13

   130   164   116   121   112   101   165   118    97   108   117   101   165

  Columns 14 through 26

   118    97   108   117   101   203     0     0     0     0     0     0     0

  Column 27

     0
msg sent.

Octave:

zeromq loaded
socket done
connection done
receiving msg:
 Columns 1 through 16:

  130  164  110   97  109  101  164  101  118   97  108  164  116  121  112  101

 Columns 17 through 27:

  170  103  101  116   95  103  108  111   98   97  108
msg received.
sending msg:
 Columns 1 through 16:

  130  164  116  121  112  101  165  118   97  108  117  101  165  118   97  108

 Columns 17 through 32:

  117  101  146  172   95   95  102  117  110   99  116  105  111  110   95   95

 Columns 33 through 37:

  164  101  118   97  108
msg sent.
receiving msg:
 Columns 1 through 16:

  132  164  110   97  109  101  164  101  118   97  108  164   97  114  103  115

 Columns 17 through 32:

  145  162   48   59  167  110   97  114  103  111  117  116  255  164  116  121

 Columns 33 through 39:

  112  101  164   99   97  108  108
msg received.
sending msg:
 Columns 1 through 16:

  130  164  116  121  112  101  165  118   97  108  117  101  165  118   97  108

 Columns 17 through 27:

  117  101  203    0    0    0    0    0    0    0    0
msg sent.
receiving msg:

@bastibe
Copy link
Owner

bastibe commented Apr 29, 2019

You might not need that. Matlab R2016b and newer have jsonencode() and jsondecode() functions now.

It does indeed, but the official implementation is slower for long strings than my implementation, and uses structs instead of containers.Map for JSON objects, which breaks JSON objects with non-variable-name keys.

@bastibe
Copy link
Owner

bastibe commented Apr 29, 2019

@kupiqu, could you print the decoded messages instead of the binary blobs?

At least it is heartening to see that my msgpack parser seems to work fine in Octave.

@kupiqu
Copy link
Author

kupiqu commented Apr 29, 2019

Unfortunately Octave crashes when I try to do so, that's why I displayed binary data:

zeromq loaded
socket done
connection done
receiving msg:
msg received.
Exception in thread Thread-1:
Traceback (most recent call last):
  File "/home/miniconda/envs/cenv1/lib/python3.7/threading.py", line 917, in _bootstrap_inner
    self.run()
  File "/home/miniconda/envs/cenv1/lib/python3.7/threading.py", line 865, in run
    self._target(*self._args, **self._kwargs)
  File "/home/miniconda/envs/cenv1/lib/python3.7/site-packages/transplant/transplant_master.py", line 127, in reader
    print(line.decode(), end='')
UnicodeDecodeError: 'utf-8' codec can't decode byte 0x82 in position 0: invalid start byte

Actually, not sure it's octave that crashes here or just python :/

@kupiqu
Copy link
Author

kupiqu commented Apr 29, 2019

Unfortunately I don't have much time at the moment to continue digging, and even if I had some I am not sure how to proceed further.

I attach the two files I changed (ZMQ.m and transplant_master.py), with the hope that you, or somebody else, can fix the remaining issues.

transplant_changes.zip

@bastibe
Copy link
Owner

bastibe commented Apr 29, 2019

Thank you very much for your efforts!

If anyone else wants to take a stab at this, I would be grateful!

@RobBW
Copy link

RobBW commented Oct 29, 2021

The message "UnicodeDecodeError: 'utf-8' codec can't decode byte 0x.." reminded me of something I encountered about 2 years ago in relation to a similar error in SMOP:
[victorlei/smop] UnicodeDecodeError (#155)
It is a generic problem with non unicode characters in a file.
See here: https://wiki.python.org/moin/UnicodeDecodeError and
"Processing Text Files in Python 3 — Nick Coghlan's Python Notes":
http://python-notes.curiousefficiency.org/en/latest/python3/text_file_processing.html
These notes point out that it is a problem arising from the Python2/3 transition and how to resolve it.
Transplant is great and adding Octave would be a big plus. Unfortunately I'm too old and unskilled to have ago at fixing the software.

@RobBW
Copy link

RobBW commented Oct 29, 2021

Just found some old notes on the UnicodeDecodeError: 'utf-8' issue.
Here's the gotcha to look out for: https://www.gsqi.com/marketing-blog/utf-8-bom-robots-txt/ concerning errors from invisible characters.
I encounterd it when trying out a variant of SMOP: https://github.com/PatrickFURI/smop which stopped when processing some m-files containing non utf-8 and invisibles .
The following links were informative too:
https://realpython.com/python-encodings-guide/
http://utf8everywhere.org

It's possible that the affected python code has been fixed since kupiqu ran his tests in 2019 and that there is nothing to repair in Transplant. One can always hope!

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

4 participants