-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Adding FastAI(pytorch) as a first class citizen to the pipeline #982
Conversation
@adricl - are you still working on this PR? It will take a bit to go through this. You should definitely make sure that the tests are passing. If you are still working on it, then you can convert the PR to |
@DocGarbanzo |
One more thing. When you are ready, can you please squash your changes to a single commit ? That way it becomes easier to reason about. |
@tikurahul to squash the commit do I have to create a new PR or can I do it in this existing PR? I am not sure how to go about it if you can do it for the existing pr. Thanks |
I think this pr is good to review now.. I can recreate this PR if required into a squash if you want. |
@adricl - you don't need to do a new PR. You can squash the commits on your branch, make sure everything is working and force-push. It will update this PR here in-place. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a great idea to bring in fastai. A couple of points need to be addressed. Please also add a test for the fastai model - you can compare how @EricWiener adapted the Keras tests to PyTorch. We should test the functionality of the model and the pipeline and the training. The latter should be switched off in the CI (we have the same setup also for TF and PyTorch).
I have to look at the tests next. That might take me a few days. |
@@ -36,6 +37,7 @@ dependencies: | |||
- plotly | |||
- pyyaml | |||
- tensorflow=2.2.0 | |||
- fastai |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add the module in setup.py
. Do you know if this installs on the RPi w/o problems?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should work as fastai is a wrapper around pytorch so there is no binaries as such. I will test this.
@adricl - this looks much better now. |
Thanks for this. I had just started working on the same idea but I haven't gotten nearly as far. |
@adricl @Bleyddyn - if you want to put records into a data frame as in the example above, this is now much easier with the new tub, because it is an Iterable over dictionaries. You just say: import pandas as pd
from donkeycar.parts.tub_v2 import Tub
t = Tub('/home/me/mycar/data')
df = pd.DataFrame(t) |
@DocGarbanzo That's what I'm doing in the notebook I linked to. Updated now with a better DataBlock definition and actual working training code. |
@Bleyddyn - just looked at your notebook now. Very good. |
I will need to remove the code around tflite and tensor rt for fastai.
@DocGarbanzo I have updated the test train pipeline to run fastai. I have also added fastai to the pi build. Is there anything else that needs to be completed for this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very good now, can you please address the small comment on the test & push it, then I'll merge the code.
We are good to go! |
Awesome contribution here ! I would like to try this out in the simulator and/or on a car. Any docs associated with this update or plan to create/modify some ? I can help review/validate the doc changes . @adricl |
Thanks, |
@c1505 this is my documentation. FastAi Docs Have a look before I merge it. |
This PR allows you to use Fast AI(pytorch) end to end in donkey car. I have tested training and driving with the simulator.
I have added a new type of model fastai_ so far linear is the only model implemented. More can be added in the future of course.
I have tried my best to emulate most of the function that Keras does but there are some idiosyncrasies between the two frameworks.
These normal training and driving commands work.
python train.py --tubs data/ --model models/fastai_linear_model.pth --type fastai_linear
python manage.py drive --model models/fastai_linear_model.pth --type fastai_linear
I do realise that this PR comes without a heads up or warning so if its rejected that's fine.
I do really like this project and the work that has been done, its just I prefer PyTorch to TensorFlow.
Any suggestions are welcome. I am new to python of this complexity so any pointers would be great.
There will be more cleanups and fixes on this PR as I am not finished yet. I need to add some graph plotting and other tidy ups and maybe a refactor. Any guidance would be great.
I will also add some docs around this.
If this goes well I might also look at getting pytorch lightning into own part and working in the pipeline.
Please note this will not work with TensorRT or TF lite if there is demand I could look at this in the future.
Cheers