-
Notifications
You must be signed in to change notification settings - Fork 4
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
Review - Improve changes and move to rag_agent #4
Conversation
Move RAG logic to a new file, use a class for the functionalities. Combine requirements.txt, no need for two separate files. Signed-off-by: Chihurumnaya Ibiam <[email protected]>
Signed-off-by: Chihurumnaya Ibiam <[email protected]>
@chimosky I'm trying to manage the dependencies in my Conda environment for my AI model, but I'm facing some challenges. When I run the pip freeze command, I see a long list of installed packages, many of which seem interconnected. This makes it difficult for me to determine which dependencies are actually required for my project. Additionally, when I installed the packages listed in my requirements.txt, many other dependencies were automatically downloaded. This is further complicating my ability to classify and understand which packages I really need. Dependencies in my conda environment
|
@chimosky @walterbender I am almost done with other changes you asked me in the TODO. I have implemented it currently using this where it checks for some keywords in this list and based on that it determines if kids query if coding based or redundant. This is just a basic layout I am planning to scale it even further but later after integration is done. If we want we can ignore this part too, let me know your opinions?
|
How do you use the keyword list? I presume it will be extended? |
It checks whether child's query contains words present in the list if yes then it answers them. Yes I think it will be extended, because for even advanced tools like chatGPT I tried opening a Python assistant and asked some non coding questions they too answered them, hence I feel advocating this method would be good in our case(as we are dealing with kids and we can't provide them redundant answers). |
Answering only coding queries doesn't make much sense to me and shouldn't be something we aim for. Like you mentioned above, what happens when there are non code related queries if we restrict the responses to just code based queries? Also, it seems you've not reviewed my changes or working based on my branch, because if you did you'd notice the Please review my changes. Also base your changes on this branch, so it'll be easier to just merge here rather than cherry-pick and push here. |
Signed-off-by: Chihurumnaya Ibiam <[email protected]>
Signed-off-by: Chihurumnaya Ibiam <[email protected]>
Yeah I skipped the changes you made on your branch I misunderstood for the main. I feel we can make this chatbot generic too I have no issue with that but the purpose why it's placed inside Pippy is that it generates code itself and not deviate from it's purpose, by code I mean that it will also provide explanation, examples and relevant code snippet. @walterbender what's your opinion on this? |
I don't understand what you mean by this, could you explain better? |
I mean that the way we have an AI-assistant in chat Activity for chatting purpose similarly this bot would be exclusively restricted to coding and python learning purpose. Bcz making it generic would loose it's value as it's placed in Pippy itself so the domain it's operating in must be specific. |
I got that part, doesn't make it a great reason to place it inside a Pippy directory, the |
@chimosky I have looked out for the dependencies of langchain they are quite updated not latest because on updating some it's giving conflicts with others, currently it's working great with python env 3.12 . I have made fixes to #6 kindly review it.
|
Reviewed, opened a PR that needs your attention. |
Signed-off-by: Chihurumnaya Ibiam <[email protected]>
@kshitijdshah99 please review e1ab999, as I've updated the dependencies with the list you sent above. |
Seems I'd already made the change in #8, I'll drop the commit. @XXXJumpingFrogXXX this would be a great time to respond to the comments in that PR. |
e1ab999
to
9a28f68
Compare
Can you add these too?
IG the nvidia ones dependencies can be excluded as of now but others are quite important. Mostly these nvidia ones get auto-downloaded with langchain dependencies. |
Signed-off-by: Chihurumnaya Ibiam <[email protected]>
I've made the changes you mentioned, please review. |
yeah it's proper |
Seems good, thanks. |
Move RAG logic to a new file, use a class
for the functionalities.
Combine requirements.txt, no need for two separate files.
TODO
self.prompt
seems to be unused, why? I'm guessing migrating from RetrievalQA might solve this.@kshitijdshah99 @XXXJumpingFrogXXX kindly review.
I'll leave the other changes to you Kshitij.
This PR is open and we'll use it to track the TODO above, and will merge once you've made the necessary changes.