Skip to content
This repository has been archived by the owner on Nov 13, 2024. It is now read-only.

Remove query generator defualt topk #66

Merged
merged 4 commits into from
Oct 15, 2023

Conversation

acatav
Copy link
Contributor

@acatav acatav commented Oct 10, 2023

Problem

We currently have two default top k and it's confusing

Solution

Remove default top k from query generator, and make it to pass None in top_k so KB will use its default. We still support passing top_k in queries to context engine, but it also possible to not pass and top_k to use the KB default

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Infrastructure change (CI configs, etc)
  • Non-code change (docs, etc)
  • None of the above: (explain here)

Test Plan

Unit tests adjusted

@acatav acatav changed the base branch from main to dev October 10, 2023 16:12
Copy link
Contributor

@igiloh-pinecone igiloh-pinecone left a comment

Choose a reason for hiding this comment

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

@acatav Approved, but please see one comment

resin/models/data_models.py Show resolved Hide resolved
@igiloh-pinecone igiloh-pinecone merged commit 89f8ac4 into dev Oct 15, 2023
9 checks passed
@igiloh-pinecone igiloh-pinecone deleted the remove-query-generator-defualt-topk branch October 15, 2023 18:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants