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

quantum mixer implementation #645

Merged
merged 8 commits into from Oct 21, 2021
Merged

Conversation

ongspxm
Copy link
Contributor

@ongspxm ongspxm commented Oct 17, 2021

closes #641

implementing a quantum class neural network from https://qiskit.org/textbook/ch-machine-learning/machine-learning-qiskit-pytorch.html

implementation notes

  • currently using QClassic wrapper around the Neural mixer
    • its easier to error out the unit test
    • because qiskit is an optional dependency
  • QClassicNet is quite rudimentary
    • returns a quantum modified value as per the tutorial

@ongspxm ongspxm changed the title quantum mixer implementation quantum mixer implementation #641 Oct 17, 2021
@ongspxm ongspxm changed the title quantum mixer implementation #641 quantum mixer implementation Oct 17, 2021
@ongspxm ongspxm force-pushed the feature/qmixer branch 2 times, most recently from 95205e8 to 92b6a35 Compare October 17, 2021 09:57
@ongspxm
Copy link
Contributor Author

ongspxm commented Oct 17, 2021

Running in /home/runner/work/lightwood/lightwood
Add input parsed as single string, running 1 git add command.
> Using 'Automated Author <info@mindsdb.com>' as author.
> Using "updating docs" as commit message.
> Running for a PR, the action will use 'feature/qmixer' as ref.
Internal logs
Outputs
Error: Error: There is no tracking information for the current branch.
Please specify which branch you want to merge with.
See git-pull(1) for details.

    git pull <remote> <branch>

If you wish to set tracking information for this branch you can do so with:

    git branch --set-upstream-to=origin/<branch> feature/qmixer

this github action seems to be failing, i ran the command locally but it doesnt seem to help resolve this issue, sorry new to this whole github action thing

@George3d6
Copy link
Contributor

There is already a PR implementing a quantum mixer here: #639

Can you please explain how it's different from it.

I don't mind merging both mixers and/or doing the work to merge them into one and award both of you 10 points for the laptop, but I'd like to understand how this is different given that it came after the original.

@ongspxm
Copy link
Contributor Author

ongspxm commented Oct 19, 2021 via email

@mrandri19
Copy link
Contributor

mrandri19 commented Oct 19, 2021

(Author of the other PR here) I have to say that the two PRs came at around the same time. This one is more production-ready since it extends the pre-existing classes rather than creating new ones, hence taking advantage of much of the existing infrastructrue. If I had to choose, I would merge this one.

I have a few comments, stemming from TODOs I have in my PR:

  • Since this reuses the Neural class, I am wondering what will happen on a regression problem, especially considering the final softmax. Perhaps this should only support classification.
  • This only supports having quantum circuits as the last layer.
  • It would be cool to experiment with more complex quantum circuits.

@George3d6
Copy link
Contributor

  • Reusing Neural is indeed a good idea assuming that the quantum network doesn't need any custom behaviour, Neural does a bunch of "fancy" logic in terms of early-stopping, switching target encoder flags, lr finding and partially fitting on the dev data, which we found helps a lot in benchmarks.
  • Personally I don't see the point of the final softmax, @ongspxm any reason why that was added?
  • As for experimenting with "more complex quantum circuits", no idea on that point

At any rate, looking at both PRs more I do like this one best, ultimately I'd want to get the mixers in and actually benchmark them.

@ongspxm any changes you'd like to make on this one or are you happy to merge as is (again, softmax as the last activation looks wrong to me but maybe I'm mistaken, idk), unittest look good so in principle, this works.

@mrandri19 would you want to finish your original PR? If so I can merge that too and the benchmark this both head-to-head and when we have the time I'll merge them into a joint implementation. Alternatively, if you have some ideas that'd build on this PR I can merge this one when @ongspxm is happy with it and then you can PR your changes into the new staging.

@ongspxm
Copy link
Contributor Author

ongspxm commented Oct 20, 2021

  • yeah, yall right, softmax should be removed. i had a brainfart and saw crossentropyloss, which made me just put it in by habit
  • added one last nit, for easy extension to other nets

yeah, i think it should be good to go

@mrandri19
Copy link
Contributor

would you want to finish your original PR? If so I can merge that too and the benchmark this both head-to-head and when we have the time I'll merge them into a joint implementation.

I think it's best to join this one, since in the current state they are doing the same thing and this one does it better.

Alternatively, if you have some ideas that'd build on this PR I can merge this one when @ongspxm is happy with it and then you can PR your changes into the new staging.

I'll do this, thanks!

@mrandri19
Copy link
Contributor

yeah, yall right, softmax should be removed. i had a brainfart and saw crossentropyloss, which made me just put it in by habit

I believe this is correct. The final softmax will be added if needed by _select_criterion:
https://github.com/mindsdb/lightwood/blob/staging/lightwood/mixer/neural.py#L77-L90

@George3d6 George3d6 merged commit 9e68d3c into mindsdb:staging Oct 21, 2021
@George3d6 George3d6 added the hacktoberfest-accepted Accept for hacktoberfest. label Oct 21, 2021
@paxcema paxcema mentioned this pull request Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Accept for hacktoberfest.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🔧 Implement a quantum mixer 🧙
3 participants