search

Tobias-Fischer / rt_gene

RT-GENE: Real-Time Eye Gaze and Blink Estimation in Natural Environments
http://www.imperial.ac.uk/personal-robotics
Other
374 stars 71 forks source link

Pytorch Model Types #107

Closed ahmed-alhindawi closed 3 years ago

ahmed-alhindawi commented 3 years ago

Hello,

Since the recent issue about model types in the blink detector, I decided that I should formalise the ability to change the model types in our pytorch code. Until this point, I was changing it manually to suit my needs but this is better...

Let me know what you think.

EDIT: While I'm here, I also added trained resnet-18 model files and uploaded the dlib face detector file too as pipeline will not work without it

Tobias-Fischer commented 3 years ago

As always many thanks for the contribution @ahmed-alhindawi! Greatly appreciated :). Just a few things to change from my review as per above.

ahmed-alhindawi commented 3 years ago

Some typos/fixes, please have a look. Have you tested that both the PyTorch and TensorFlow versions still run fine? Also, I don't understand the dlib model addition. Did the code never work previously or why is it needed now?

Thanks for your review! The TensorFlow and the PyTorch versions do run nicely.

The models for dlib weren't distributable before - their license meant that pre-trained networks had to be downloaded directly from their website. However, it seems that they had a change of heart and now "you can do whatever you like with them"; thus I've added it to our code to download automatically. And yes, it wouldn't work before...

Lastly - I think my commits are in master rather than in the new branch. Apologies if that's the case! Unintentional...

SohilZidan commented 3 years ago

Hi @ahmed-alhindawi, these lines should contain the model_type argument: https://github.com/Tobias-Fischer/rt_gene/blob/cd4af5f288cefaa6259d4cff4ed72c84fee0c4dc/rt_bene_standalone/estimate_blink_standalone.py#L92 and https://github.com/Tobias-Fischer/rt_gene/blob/cd4af5f288cefaa6259d4cff4ed72c84fee0c4dc/rt_bene_standalone/estimate_blink_standalone.py#L95

ahmed-alhindawi commented 3 years ago

Hi @ahmed-alhindawi, these lines should contain the model_type argument:

https://github.com/Tobias-Fischer/rt_gene/blob/cd4af5f288cefaa6259d4cff4ed72c84fee0c4dc/rt_bene_standalone/estimate_blink_standalone.py#L92

and

https://github.com/Tobias-Fischer/rt_gene/blob/cd4af5f288cefaa6259d4cff4ed72c84fee0c4dc/rt_bene_standalone/estimate_blink_standalone.py#L95

You're quite right - done that too. Let me know if you have any questions/issues.

Tobias-Fischer commented 3 years ago

Did one minor change in master - closing the PR as it pulls the wrong way ;).

Thanks again @ahmed-alhindawi and thanks for checking the PR @SohilZidan