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

docs(tutorial): polish image2image #3843

Merged
merged 4 commits into from
Nov 2, 2021

Conversation

alexcg1
Copy link
Member

@alexcg1 alexcg1 commented Nov 1, 2021

Just polishing up the wording a little bit

@alexcg1 alexcg1 self-assigned this Nov 1, 2021
@github-actions github-actions bot added size/M area/docs This issue/PR affects the docs labels Nov 1, 2021
@alexcg1
Copy link
Member Author

alexcg1 commented Nov 1, 2021

Did we want to explain the indexer here? Instead we're just directing people to our concept docs

covered. In Jina we have three fundamental concepts that you need to know to follow this tutorial. If you
haven't read it yet, head on over to [Jina's basic concepts](https://docs.jina.ai/fundamentals/concepts/) page. 

I'd suggest that instead of this, we can just link the first occurrence of Document, Executor and Flow to their respective docs page

@alexcg1
Copy link
Member Author

alexcg1 commented Nov 1, 2021

Did we want to explain the indexer here? Instead we're just directing people to our concept docs

covered. In Jina we have three fundamental concepts that you need to know to follow this tutorial. If you
haven't read it yet, head on over to [Jina's basic concepts](https://docs.jina.ai/fundamentals/concepts/) page. 

I'd suggest that instead of this, we can just link the first occurrence of Document, Executor and Flow to their respective docs page

And/or mention that readers should check the basic concepts before they start

@makram93
Copy link
Member

makram93 commented Nov 1, 2021

I think we can remove the explanation. They would have gone through the basics before this tutorial

@codecov
Copy link

codecov bot commented Nov 1, 2021

Codecov Report

Merging #3843 (800d7d0) into master (a66b4aa) will decrease coverage by 5.55%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3843      +/-   ##
==========================================
- Coverage   86.14%   80.59%   -5.56%     
==========================================
  Files         156      156              
  Lines       12213    12223      +10     
==========================================
- Hits        10521     9851     -670     
- Misses       1692     2372     +680     
Flag Coverage Δ
daemon ?
jina 79.13% <ø> (-6.47%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
jina/helloworld/chatbot/my_executors.py 0.00% <0.00%> (-95.56%) ⬇️
jina/helloworld/fashion/my_executors.py 0.00% <0.00%> (-93.16%) ⬇️
jina/helloworld/fashion/helper.py 0.00% <0.00%> (-90.82%) ⬇️
jina/helloworld/chatbot/app.py 0.00% <0.00%> (-83.68%) ⬇️
jina/helloworld/fashion/app.py 0.00% <0.00%> (-81.82%) ⬇️
jina/hubble/hubio.py 9.84% <0.00%> (-78.42%) ⬇️
jina/hubble/hubapi.py 20.00% <0.00%> (-75.24%) ⬇️
jina/hubble/helper.py 23.24% <0.00%> (-67.57%) ⬇️
jina/parsers/deprecated.py 33.33% <0.00%> (-50.01%) ⬇️
jina/clients/helper.py 50.00% <0.00%> (-50.00%) ⬇️
... and 49 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a66b4aa...800d7d0. Read the comment docs.

@alexcg1
Copy link
Member Author

alexcg1 commented Nov 1, 2021

I think we can remove the explanation. They would have gone through the basics before this tutorial

We live in hope

@github-actions
Copy link

github-actions bot commented Nov 1, 2021

📝 Docs are deployed on https://docs-tutorial-similar-image-polish--jina-docs.netlify.app 🎉

1 similar comment
@github-actions
Copy link

github-actions bot commented Nov 1, 2021

📝 Docs are deployed on https://docs-tutorial-similar-image-polish--jina-docs.netlify.app 🎉

@github-actions
Copy link

github-actions bot commented Nov 1, 2021

Latency summary

Current PR yields:

  • 😶 index QPS at 1094, delta to last 2 avg.: -2%
  • 😶 query QPS at 52, delta to last 2 avg.: +0%
  • 😶 dam extend QPS at 35477, delta to last 2 avg.: -15%
  • 😶 avg flow time within 1.3914 seconds, delta to last 2 avg.: +0%
  • 😶 import jina within 0.4654 seconds, delta to last 2 avg.: -1%

Breakdown

Version Index QPS Query QPS DAM Extend QPS Avg Flow Time (s) Import Time (s)
current 1094 52 35477 1.3914 0.4654
2.2.2 1083 50 38650 1.1651 0.5306
2.2.1 1157 53 45046 1.5964 0.417

Backed by latency-tracking. Further commits will update this comment.

Comment on lines +32 to +33
We will use the **SimpleIndexer** Executor as
our indexer (the one that stores and retrieves data). This Executor also returns the matching `Document` when we make
Copy link
Member

@makram93 makram93 Nov 1, 2021

Choose a reason for hiding this comment

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

Suggested change
We will use the **SimpleIndexer** Executor as
our indexer (the one that stores and retrieves data). This Executor also returns the matching `Document` when we make
The **Indexer** executor support vector search along with indexing. For this tutorial we will create our own **Indexer** based on `SimpleIndexer` executor available on Jina Hub. This Executor returns the matching `Document` when we make

Copy link
Member Author

Choose a reason for hiding this comment

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

Automated message

Capitalization

Please capitalize first letters of the following (when used to reference project or component):

  • Flow
  • Executor
  • Document
  • Jina
  • Jina Hub
  • Python
  • Docker
  • Kubernetes

Note!

This only applies outside a code or CLI context. If you're describing Python, YAML, any programming language, or command line input/output, please use the capitalization you would use in that context.

Right
from jina.flow import Flow
Wrong
from Jina.Flow import Flow

Made with GitHub saved replies

Copy link
Member Author

Choose a reason for hiding this comment

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

Remove "should"

getting the embeddings. Reader can replace this model with any other pre-trained models of their choice. When this
executor is instantiated, the pre-trained weights are downloaded automatically. The `predict` function takes in
the `DocumentArray` and extracts embeddings which is then stored in the `embedding` attribute of the
To build an Encoder Executor we inherit the base `Executor` and use a decorator
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
To build an Encoder Executor we inherit the base `Executor` and use a decorator
To build an `Encoder` Executor we inherit the base `Executor` and use a decorator

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we need to backtick Encoder or Executor here. We're talking about them as concepts, not code

Comment on lines +90 to +93
Finally, comes the storage/retrieval step. We do this with the **Indexer** Executor. You can use any of the
available indexers on [Jina Hub](https://hub.jina.ai) or define your own. To create an **Indexer** you need to have two
endpoints: `/index` and `/search`. For this tutorial we will define a `SimpleIndexer` which is [also available on jina
Hub](https://hub.jina.ai/executor/zb38xlt4).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Finally, comes the storage/retrieval step. We do this with the **Indexer** Executor. You can use any of the
available indexers on [Jina Hub](https://hub.jina.ai) or define your own. To create an **Indexer** you need to have two
endpoints: `/index` and `/search`. For this tutorial we will define a `SimpleIndexer` which is [also available on jina
Hub](https://hub.jina.ai/executor/zb38xlt4).
Finally, comes the storage/retrieval step. You can use any of the
available indexers on [Jina Hub](https://hub.jina.ai) or define your own. To create an **Indexer** you need to have two
endpoints: `/index` and `/search`. We can create an Executor which supports vector search and indexing as follows:

@hanxiao hanxiao merged commit 5905c85 into master Nov 2, 2021
@hanxiao hanxiao deleted the docs-tutorial-similar-image-polish branch November 2, 2021 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs This issue/PR affects the docs size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants