-
Notifications
You must be signed in to change notification settings - Fork 2
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 total number of runs to logs & small fixes #72
Conversation
src/parameter_search.ipynb
Outdated
" 'chunk_size' : [192, 256, 320],\n", | ||
" 'chunk_overlap' : [64, 128, 192],\n", | ||
" 'chunk_size' : [128, 192, 256, 320],\n", | ||
" 'chunk_overlap' : [32, 64, 96, 128, 160, 192],\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a risk that this will run for a very long time?
I think the best balance might be in providing few values with large intervals between them e.g. [32, 128, 192] in order to show the contrast in the final score with a relatively short time to complete. We want to minimize the time of wait + API costs for the end user
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree
src/parameter_search.ipynb
Outdated
"}\n", | ||
"searchable_shared_params = {\n", | ||
" 'embed_model' : [(\"huggingface\",\"all-MiniLM-L6-v2\")]\n", | ||
" 'embed_model' : [(\"huggingface\",\"all-MiniLM-L6-v2\"),(\"huggingface\", \"BAAI/bge-base-en-v1.5\")]\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we re-introducing the bge
model again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will add it as a comment
src/parameter_search.ipynb
Outdated
@@ -417,6 +419,7 @@ | |||
" search_method = search_method,\n", | |||
" target_metric = target_metric,\n", | |||
" evaluation_set_path=evaluation_set_path,\n", | |||
" debug=True,\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be false I suppose, or do we leave it on for the end user
src/utils.py
Outdated
check_weaviate_class_exists( | ||
weaviate_client, | ||
indexing_config["weaviate_class"], | ||
) | ||
|
||
logger.info( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probable be above line 190 before we start the run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense
) | ||
|
||
logger.info( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Jan :)
No description provided.