cookbook icon indicating copy to clipboard operation
cookbook copied to clipboard

Improving object detection colab

Open Giom-V opened this issue 1 year ago • 23 comments
trafficstars

  • 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 avatar Jul 11 '24 20:07 Giom-V

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

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!

View / edit / reply to this conversation on ReviewNB

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

Typo: examples


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.

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

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.

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

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.

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

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

markmcd avatar Aug 28 '24 06:08 markmcd

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


View entire conversation on ReviewNB

Giom-V avatar Sep 11 '24 12:09 Giom-V

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

Giom-V avatar Sep 11 '24 12:09 Giom-V

Fixed


View entire conversation on ReviewNB

Giom-V avatar Sep 11 '24 12:09 Giom-V

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


View entire conversation on ReviewNB

Giom-V avatar Sep 11 '24 12:09 Giom-V

Fixed


View entire conversation on ReviewNB

Giom-V avatar Sep 11 '24 12:09 Giom-V

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


View entire conversation on ReviewNB

Giom-V avatar Sep 11 '24 12:09 Giom-V

Fixed


View entire conversation on ReviewNB

Giom-V avatar Sep 11 '24 12:09 Giom-V

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

Giom-V avatar Sep 11 '24 13:09 Giom-V

Replaced with a new picture from wikipedia.


View entire conversation on ReviewNB

Giom-V avatar Sep 11 '24 13:09 Giom-V

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

markmcd avatar Sep 12 '24 08:09 markmcd

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

markmcd avatar Sep 12 '24 08:09 markmcd

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.

I updated the outputs. It should be better now.


View entire conversation on ReviewNB

Giom-V avatar Sep 13 '24 09:09 Giom-V