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.

Sign up for GitHub

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

Jump to bottom

Implement a tool to query ORAssistant #6226

Open
wants to merge 42 commits into
base: master
Choose a base branch
from
Open

Implement a tool to query ORAssistant #6226

wants to merge 42 commits into from

Conversation

palaniappan-r
Copy link

Implemented the askbot and set_bothost commands under the ora tool. They can be used to make queries to ORAssistant, a LLM based conversational assistant for OpenROAD

askbot

This command takes an argument and passes it as a query to the hosted ORAssistant backend, by making a POST request using the libcurl library.

set_bothost

This command modifies ORAssistant backend's URL that's being queried by askbot. The modified URL is then saved to a file locally, for subsequent reference.

palaniappan-r and others added 25 commits October 21, 2024 14:36
@palaniappan-r
ora: add basic orassistant api call command
fdd34df 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: add boost json lib
13453ac 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: remove context flag
ad67b3a 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: remove context flag, use boost json to parse response
59108fd 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: remove context flag
c03c44d 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: install boost json in docker container
0dd9809 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: add tool entry in logger
972e8d8 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: add set_bothost command
3608133 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: save bothost url to file
11e1fb4 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
Merge branch 'The-OpenROAD-Project:master' into master
581f947
@palaniappan-r
ora: update dockerfile
a885998 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: revert builder dockerfile
68096d7 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: add curl to dependency installer script
119617e 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: add curl and openssl to dependency installer script
ec4c653 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
style: fix formatting
a5c7e96 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: remove default tests
a167887 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
Merge branch 'The-OpenROAD-Project:master' into master
504d82c
@palaniappan-r
Merge branch 'The-OpenROAD-Project:master' into master
fb30cf7
@palaniappan-r
ora: remove log statement
4cf083c 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: add tests
72de8cd 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: add mock API server initialization in regression tests
e64dd2b 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: replace FastAPI mock server with HTTPServer implementation
68e8156 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: add regression test script definition for 'ora' tool
056562f 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: update copyright year to 2024
c44816c 
Signed-off-by: Palaniappan R <[email protected]>
@palaniappan-r
ora: update copyright year to 2024 and remove host url file
d2ac50f 
Signed-off-by: Palaniappan R <[email protected]>
@rovinski
Copy link
Collaborator

Why can't this run locally as an add-on rather than sending requests over the internet?

@palaniappan-r
Copy link
Author

Why can't this run locally as an add-on rather than sending requests over the internet?

If the user chooses to the application locally, they'll have to follow instructions on ORAssistant's README, spin the backend server up, and use the set_bothost command to have the askbot command query the local server instead.

Currently, askbot queries a hosted version of ORAssistant by default, but I can change this if necessary.

@rovinski
Copy link
Collaborator

Is there a data control plan or disclaimer if users are sending data over the internet? GDPR may even come into play here.

@palaniappan-r
Copy link
Author

palaniappan-r commented Nov 27, 2024 •
edited
Loading

Is there a data control plan or disclaimer if users are sending data over the internet? GDPR may even come into play here.

The only data being transmitted over the internet is the query entered by the user.

Currently, there is no formal disclaimer in place. I could draft a suitable disclaimer to address this and ensure compliance with relevant regulations such as GDPR.

@palaniappan-r palaniappan-r marked this pull request as draft December 5, 2024 15:17
@palaniappan-r palaniappan-r marked this pull request as ready for review December 7, 2024 23:01
@palaniappan-r palaniappan-r marked this pull request as draft December 18, 2024 00:23
@palaniappan-r palaniappan-r marked this pull request as ready for review December 21, 2024 03:14
luarss
luarss reviewed Dec 24, 2024
View reviewed changes
Copy link
Contributor

@luarss luarss left a comment

Choose a reason for hiding this comment

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

Just some nits, no comments on code end

docker/Dockerfile.builder Outdated
@@ -18,4 +18,4 @@ WORKDIR /OpenROAD

ENV PATH=${LOCAL_PATH}:${PATH}

RUN ./etc/Build.sh -compiler=${compiler} -threads=${numThreads} -deps-prefixes-file=${depsPrefixFile}
RUN ./etc/Build.sh -compiler=${compiler} -threads=${numThreads} -deps-prefixes-file=${depsPrefixFile}
Copy link
Contributor

Choose a reason for hiding this comment

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

why is there a change here?

Copy link
Author

Choose a reason for hiding this comment

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

Reverted.

etc/DependencyInstaller.sh
@@ -579,7 +583,7 @@ _installDebianPackages() {
unzip \
wget \
zlib1g-dev

Copy link
Contributor

Choose a reason for hiding this comment

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

stray change?

src/ora/src/Ora.tcl Outdated
@@ -0,0 +1,43 @@
# Copyright (c) 2024, The Regents of the University of California
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe Copyright 2025?

Copy link
Author

Choose a reason for hiding this comment

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

I've updated it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luarss luarss luarss left review comments

@maliberty maliberty Awaiting requested review from maliberty

@vvbandeira vvbandeira Awaiting requested review from vvbandeira

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@palaniappan-r @rovinski @luarss