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

Add notebook for semantic reranking with HF Eland model #314

Merged
merged 3 commits into from
Aug 14, 2024

Conversation

leemthompo
Copy link
Contributor

@leemthompo leemthompo commented Aug 13, 2024

Notebook for semantic reranking using retriever + model uploaded via Eland

Test it in Colab

Open In Colab

@leemthompo leemthompo self-assigned this Aug 13, 2024
Copy link

gitnotebooks bot commented Aug 13, 2024

Found 1 changed notebook. Review the changes at https://gitnotebooks.com/elastic/elasticsearch-labs/pull/314

@jeffvestal
Copy link
Contributor

Enabling telemetry step throws an error, referencing es_client before it is created. Should you be passing client ?

[<ipython-input-5-738307b21c1d>](https://localhost:8080/#) in <cell line: 4>()
      2 from telemetry import enable_telemetry
      3 
----> 4 es_client = enable_telemetry(es_client, "11-semantic-reranking-hugging-face")

NameError: name 'es_client' is not defined```

@jeffvestal
Copy link
Contributor

eland_import_hub_model: error: argument --cloud-id: expected one argument
also an error with ES_API_KEY

should be

!eland_import_hub_model \
  --cloud-id $ELASTIC_CLOUD_ID \
  --es-api-key $ELASTIC_API_KEY \
  --hub-model-id cross-encoder/ms-marco-MiniLM-L-6-v2 \
  --task-type text_similarity \
  --clear-previous \
  --start

@jeffvestal
Copy link
Contributor

I think you need to create an Inference endpoint after uploading the reranking model.
The last query in the notebook throws

  "error": {
    "root_cause": [],
    "type": "search_phase_execution_exception",
    "reason": "Computing updated ranks for results failed",
    "phase": "rank-feature",
    "grouped": true,
    "failed_shards": [],
    "caused_by": {
      "type": "resource_not_found_exception",
      "reason": "Inference endpoint not found [my-msmarco-minilm-model]"
    }
  },
  "status": 404
}```

Copy link
Contributor

@demjened demjened left a comment

Choose a reason for hiding this comment

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

Please add the missing step and check the other comments.

"name": "stdout",
"output_type": "stream",
"text": [
"\u001b[?25l \u001b[90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\u001b[0m \u001b[32m0.0/480.2 kB\u001b[0m \u001b[31m?\u001b[0m eta \u001b[36m-:--:--\u001b[0m\r\u001b[2K \u001b[91m━━━━━━━\u001b[0m\u001b[91m╸\u001b[0m\u001b[90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\u001b[0m \u001b[32m92.2/480.2 kB\u001b[0m \u001b[31m4.0 MB/s\u001b[0m eta \u001b[36m0:00:01\u001b[0m\r\u001b[2K \u001b[91m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\u001b[0m\u001b[90m╺\u001b[0m \u001b[32m471.0/480.2 kB\u001b[0m \u001b[31m7.5 MB/s\u001b[0m eta \u001b[36m0:00:01\u001b[0m\r\u001b[2K \u001b[90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\u001b[0m \u001b[32m480.2/480.2 kB\u001b[0m \u001b[31m5.1 MB/s\u001b[0m eta \u001b[36m0:00:00\u001b[0m\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

From what I've seen in other notebooks, in general we omit unnecessary output and only display the output of stages that showcase the current feature (in this case the before/after search results). This makes the notebook cleaner and easier to follow.
You can do this by clearing all cell outputs in your notebook tool (in VSCode there is a button on top), and replaying only the steps you want to see outputs of.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

100% I deleted what I think is noise, bu it can be helpful to have some outputs (like the eland step) for error prone steps so users can see what 'success' looks like

notebooks/search/11-semantic-reranking-hugging-face.ipynb Outdated Show resolved Hide resolved
" }\n",
" },\n",
" \"field\": \"plot\",\n",
" \"inference_id\": \"my-msmarco-minilm-model\",\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we're missing a step in the notebook, the one that creates the rerank inference endpoint pointing to the uploaded MSMarco model.

@demjened demjened self-requested a review August 13, 2024 21:12
Copy link
Contributor

@demjened demjened left a comment

Choose a reason for hiding this comment

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

LGTM. Not sure why the CI is failing, looks unrelated to this notebook.

@leemthompo leemthompo merged commit 2b6aba8 into elastic:main Aug 14, 2024
2 of 5 checks passed
@leemthompo leemthompo deleted the rerank-hugging-face-eland branch August 14, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants