-
Notifications
You must be signed in to change notification settings - Fork 75
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
Provide bindings for BaseEncoder for python encoders #704
Comments
@dkeeney would you mind looking at this, when you have time, please? |
Ok, I will take a look this weekend when I return. |
I am looking to see what to do next and this seems like a reasonable choice. Here are the things I am considering, in no particular order:
Or, anything else that seems more important? Comments anyone? |
Well, it looks like I will be doing the pickle fix next instead. Issue #732 |
Of those ideas, for me..
this would be a nice to have 👍 a visualization of SDR + time-series plots. Although we don't want to get too complicated, there are other great tools for that here..hinting at the HTMpandaVis (?) tool discussed at the forums.
I think this would be great! Followed by
although not sure if feasible.
👍
I'm not sure we need this, since
personally, #714 is annoying me 🗡️ my 2c, thanks for your amazing work, David!! 💯 |
Thanks for helping me prioritize. I do need to fix the pickle thing for @fcr first. #714 needs to be fixed too. As you know I don't do much Python work but this caught me once as well. The base class for python encoders project should be done by someone with lots of python experience although I can give a first cut at it. To work well with an EncoderRegion it should accept its arguments as **kwargs so they can be passed through from the Yaml parser. Other than that I guess its just putting common stuff in the base class. |
it is needed for tuning/understanding the models in RL problems. But thinking about it, not sure if there's a good enough c++ toolkit (opencv2 would likely do the job, but isn't that too much to include as a dep?), so we might want that only in python (?)..but that I'm thinking too much ahead, let's discuss when it's on plate. |
Agreed. I am thinking that for Python we have a nice tool already. But for C++ and eventually C# apps trying out the HotGym example, we have nothing. opencv2 is too much. I was thinking we should be able to find a simple plotting tool for C++ someplace. |
on the other take, cv2 would be well supported and ported to all platforms |
@dkeeney when you have time, would you like looking at this issue? |
@breznak There seems to be more than one issue here. The title talks about py bindings for BaseEncoder. The text is talking about a plot package for C++. I assume we need both. |
I'd keep this issue for py bindings for BaseEncoder. plot tool (region?) might be useful, but we should check with exiting tools, ie with HTMPandaVis from @Zbysekz etc.. but let's keep that for a separate issue -> #748 |
Ok, I will see what I can do for a binding for the BaseEncoder. |
The BaseEncoder class has no code. It is basically a template to force C++ sub classes to have serialization, dimensions, a reset() function, and an encode( ) function. I don't know how to force Python code to do that since it does not have virtual functions like those in C++. @breznak do you have any ideas? |
I think you can do
or there's ABCMeta But I don;t know not to overcomplicate too much? |
Yes, but that would not be written in C++ as a binding but rather as pure Python. |
So py-only encoders can derive from that class (same as c++).
Needed for: #691 , #259
The text was updated successfully, but these errors were encountered: