6 separate fit transform in separated bit plan encoder

lightonai / lightonml

Python library for running large-scale computations on LightOn's OPUs

https://docs.lighton.ai

Other

35 stars 11 forks source link

6 separate fit transform in separated bit plan encoder #7

Closed lucgiffon closed 2 years ago

lucgiffon commented 2 years ago

This PR is WIP, I submit it so that we can review together. I don't know if that is the usual way to contribute.

First commit is a very small change to fix a syntax error.
Second commit contains the core changes. You'll see that there are todos in the code where I don't understand very well the existing code. In a nutshell, there are case where some information might be lost in the encoding process. This is not related to the changes I've made. I just noticed it when I implemented the warning condition.

I am not in a hurry so don't worry if this is low priority for you. Bye :)

lucgiffon commented 2 years ago

I have found some issue so I closed the PR. I need to do more testing.

I will come back to you at some point but if it takes too long, I will bypass this issue and try an other approach to achieve my goal.

lucgiffon commented 2 years ago

Ok, I fixed the issue I believe. But this makes me think that I should write a set of unittest to make sure everything is clean. If you want me to do that, I can but I don't see any tests directory. I believe you have one on your private repository. What is the test suite that you are using? Where should I put mine?

iacolippo commented 2 years ago

Nice, sure. We use Python's unittest, you could place them in a test_public directory to avoid any unwanted interaction with the private tests on our side.