Improve notebook #44
Improve notebook #44
Conversation
| @@ -70,5 +70,5 @@ fondant --help | |||
|
|
|||
| There are two options to run the pipeline: | |||
|
|
|||
| - [Via python files and the Fondant CLI](./src/README.md): how you should run Fondant in production | |||
There was a problem hiding this comment.
This did not point to anything
There was a problem hiding this comment.
Seems like I deleted it in this PR.
It might make sense to re-add it just for the indexing pipeline. WDYT?
If not, I would still add a link for the CLI to the documentation, and keep the link to the notebook.
There was a problem hiding this comment.
Might be best to go for the second approach since we don't have a file ready to launch the pipeline from (currently organized in a function that creates the pipeline). Updated
PhilippeMoussalli
force-pushed
the
improve-notebook
branch
from
7c17345 to
603868d
Compare
PhilippeMoussalli
requested review from
RobbeSneyders,
mrchtr and
Hakimovich99
There was a problem hiding this comment.
Thanks @PhilippeMoussalli
Could you remove the notebook outputs from git by running this command?
git config filter.strip-notebook-output.clean 'jupyter nbconvert --ClearOutputPreprocessor.enabled=True --to=notebook --stdin --stdout --log-level=ERROR' Github won't even show me the diffs because they are too large 😅
| @@ -70,5 +70,5 @@ fondant --help | |||
|
|
|||
| There are two options to run the pipeline: | |||
|
|
|||
| - [Via python files and the Fondant CLI](./src/README.md): how you should run Fondant in production | |||
There was a problem hiding this comment.
Seems like I deleted it in this PR.
It might make sense to re-add it just for the indexing pipeline. WDYT?
If not, I would still add a link for the CLI to the documentation, and keep the link to the notebook.
| @@ -3,7 +3,7 @@ services: | |||
| weaviate: | |||
| image: semitechnologies/weaviate:1.20.5 | |||
| ports: | |||
| - 8080:8080 | |||
| - 8081:8080 | |||
There was a problem hiding this comment.
Is there a reason for this change?
There was a problem hiding this comment.
Port 8080 is occupied when using jupyter on vertex workbench
src/evaluation.ipynb
Outdated
There was a problem hiding this comment.
Can't see the diff on Github, but from inspecting it locally, I think you inserted some images inline which doesn't work well. Can you add them as separate images to the repo and reference them by link in the notebook like we do for the other images?
There was a problem hiding this comment.
I think I embedded them because for some reason they are not rendered properly neither in the IDE visualizer nor when you run them with the local jupyter notebook
This is an example from the parameter search notebook, I launched the notebook command from the src directory
There was a problem hiding this comment.
I see. It only works when starting from the root directory indeed.
src/evaluation.ipynb
Outdated
There was a problem hiding this comment.
Can't add the comment on the line.
I think this line:
"evaluation_llm_kwargs": {"openai_api_key": os.environ["OPENAI_KEY"], model_name : "gpt-3.5-turbo"} should be changed into:
"evaluation_llm_kwargs": {"openai_api_key": os.environ["OPENAI_KEY"], "model_name" : "gpt-3.5-turbo"}.
There was a problem hiding this comment.
Nice catch, updated
No description provided.