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

Improving object detection colab #240

Merged
merged 26 commits into from
Nov 15, 2024
Merged

Conversation

Giom-V
Copy link
Collaborator

@Giom-V Giom-V commented Jul 11, 2024

  • fixing the 4th example by removing the count and the parsing that was depending on Gemini's output
  • Adding a comment about lowering the safety features (to be discussed if it works without lowering the safety features)
  • Simplifying the Open vocabulary object detection part

Giom-V and others added 4 commits June 20, 2024 15:01
* fixing the 4th exemple by removing the count and the parsing that was depending on Gemini's output
* Adding a comment about lowerering the safety features
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added status:awaiting review PR awaiting review from a maintainer component:examples Issues/PR referencing examples folder labels Jul 11, 2024
Copy link

review-notebook-app bot commented Aug 28, 2024

View / edit / reply to this conversation on ReviewNB

markmcd commented on 2024-08-28T06:28:52Z
----------------------------------------------------------------

Can you change the references to "Gemini" to say "Gemini API"? Gemini is the consumer-facing chatbot, it's confusing, so we need to be clear (it's something marketing legal request we do).

(as a side note, I wonder if we could use structured JSON output schemas to make the output parsing simpler and stable. just a thought, not something for this change)


Giom-V commented on 2024-09-11T12:46:11Z
----------------------------------------------------------------

Done for the references to Gemini. I will think of structured output, it's a good idea.

Giom-V commented on 2024-09-11T13:53:04Z
----------------------------------------------------------------

For the structured output, since this PR is already complicated, I propose to do it in a second one if that's ok with you.

markmcd commented on 2024-09-12T08:09:47Z
----------------------------------------------------------------

do it in a second one if that's ok with you.

yeah for sure - sorry, I was just thinking out loud!

Copy link

review-notebook-app bot commented Aug 28, 2024

View / edit / reply to this conversation on ReviewNB

markmcd commented on 2024-08-28T06:28:52Z
----------------------------------------------------------------

Typo: examples


Copy link

review-notebook-app bot commented Aug 28, 2024

View / edit / reply to this conversation on ReviewNB

markmcd commented on 2024-08-28T06:28:53Z
----------------------------------------------------------------

Line #2.    # Adapted from https://colab.research.google.com/drive/1bjGbrtjE_Ugc3YpIvQMtkqsiMPqIp89W#scrollTo=02GJS3Zv4JAu  written by Andreas Steiner and Gabriel Antoine le Roux

If this work is derived from that link, you'll need to include their copyright line in the header at the top of this file along with ours.


Giom-V commented on 2024-09-11T12:51:41Z
----------------------------------------------------------------

Do you have an example for that? I haven't changed this part of the code but I agree we should do it right...

markmcd commented on 2024-09-12T08:21:00Z
----------------------------------------------------------------

It may be sufficient to just include the copyright line from that Drive URL at the top of our file but I'll send an email to people who can answer with authority. I'll CC you.

Copy link

review-notebook-app bot commented Aug 28, 2024

View / edit / reply to this conversation on ReviewNB

markmcd commented on 2024-08-28T06:28:54Z
----------------------------------------------------------------

Nit: Let's is first-person. Now ask the model... works fine, and Also ask for ...


Giom-V commented on 2024-09-11T12:52:10Z
----------------------------------------------------------------

Fixed

Copy link

review-notebook-app bot commented Aug 28, 2024

View / edit / reply to this conversation on ReviewNB

markmcd commented on 2024-08-28T06:28:55Z
----------------------------------------------------------------

Line #1.    url = "https://github.com/geminidemospatial/SpatialDemo/blob/main/gem_dogs.png?raw=true"

What are these images? There's no license in the repo, so the owner retains copyright, and we are not permitted to distribute them (e.g. by embedding them in a notebook and uploading to github)

If they are a Googler you could reach out and ask them to document the repo properly so we can use it?


Giom-V commented on 2024-09-11T12:54:14Z
----------------------------------------------------------------

I have absolutely no idea who it is, so I guess I'll have to change the pictures.

Giom-V commented on 2024-09-11T13:53:25Z
----------------------------------------------------------------

Replaced with a new picture from wikipedia.

Copy link

review-notebook-app bot commented Aug 28, 2024

View / edit / reply to this conversation on ReviewNB

markmcd commented on 2024-08-28T06:28:55Z
----------------------------------------------------------------

Nits:

  • Drop our (first person), and I 
  • Instead of saying newest, refer to the specific model (so the content is timeless)


Giom-V commented on 2024-09-11T12:55:22Z
----------------------------------------------------------------

Fixed

Copy link

review-notebook-app bot commented Aug 28, 2024

View / edit / reply to this conversation on ReviewNB

markmcd commented on 2024-08-28T06:28:56Z
----------------------------------------------------------------

If we're distributing these images (e.g. by including them in a notebook we upload to GitHub), we need to comply with their license (which appears to be MIT?).


Giom-V commented on 2024-09-11T12:56:46Z
----------------------------------------------------------------

Not sure what it entails to be honest. I'll talk to you directly about that.

Copy link

review-notebook-app bot commented Aug 28, 2024

View / edit / reply to this conversation on ReviewNB

markmcd commented on 2024-08-28T06:28:57Z
----------------------------------------------------------------

Nit: Gemini API discovery


Giom-V commented on 2024-09-11T12:56:53Z
----------------------------------------------------------------

Fixed

@markmcd
Copy link
Member

markmcd commented Aug 28, 2024

Left some review feedback - mostly little nits but there are a couple of licensing questions we need to answer too.

@markmcd markmcd added status:awaiting response Awaiting a response from the author and removed status:awaiting review PR awaiting review from a maintainer labels Aug 28, 2024
Copy link

@Alexisloyolamend3z Alexisloyolamend3z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gracias

Copy link
Collaborator Author

Giom-V commented Sep 11, 2024

Done for the references to Gemini. I will think of structured output, it's a good idea.


View entire conversation on ReviewNB

Copy link
Collaborator Author

Giom-V commented Sep 11, 2024

Do you have an example for that? I haven't changed this part of the code but I agree we should do it right...


View entire conversation on ReviewNB

Copy link
Collaborator Author

Giom-V commented Sep 11, 2024

Fixed


View entire conversation on ReviewNB

Copy link
Collaborator Author

Giom-V commented Sep 11, 2024

I have absolutely no idea who it is, so I guess I'll have to change the pictures.


View entire conversation on ReviewNB

Copy link
Collaborator Author

Giom-V commented Sep 11, 2024

Fixed


View entire conversation on ReviewNB

Copy link
Collaborator Author

Giom-V commented Sep 11, 2024

Not sure what it entails to be honest. I'll talk to you directly about that.


View entire conversation on ReviewNB

Copy link
Collaborator Author

Giom-V commented Sep 11, 2024

Fixed


View entire conversation on ReviewNB

Copy link
Collaborator Author

Giom-V commented Sep 11, 2024

For the structured output, since this PR is already complicated, I propose to do it in a second one if that's ok with you.


View entire conversation on ReviewNB

Copy link
Collaborator Author

Giom-V commented Sep 11, 2024

Replaced with a new picture from wikipedia.


View entire conversation on ReviewNB

Copy link
Member

markmcd commented Sep 12, 2024

do it in a second one if that's ok with you.

yeah for sure - sorry, I was just thinking out loud!


View entire conversation on ReviewNB

Copy link
Member

markmcd commented Sep 12, 2024

It may be sufficient to just include the copyright line from that Drive URL at the top of our file but I'll send an email to people who can answer with authority. I'll CC you.


View entire conversation on ReviewNB

Copy link

review-notebook-app bot commented Sep 12, 2024

View / edit / reply to this conversation on ReviewNB

markmcd commented on 2024-09-12T08:29:30Z
----------------------------------------------------------------

This cell has a weird error output, and the next one has an error too. Make sure they get cleaned up before merging.


Giom-V commented on 2024-09-13T09:42:34Z
----------------------------------------------------------------

I updated the outputs. It should be better now.

Copy link
Collaborator Author

Giom-V commented Sep 13, 2024

I updated the outputs. It should be better now.


View entire conversation on ReviewNB

@Giom-V Giom-V requested review from markmcd and MarkDaoust and removed request for MarkDaoust September 16, 2024 09:46
@Giom-V Giom-V merged commit 6916e86 into google-gemini:main Nov 15, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:examples Issues/PR referencing examples folder status:awaiting response Awaiting a response from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants