[python] Created `get_parameters` API #668

rossdanlm · 2023-12-29T03:49:30Z

[python] Created get_parameters API

Disclaimer: I know this is 400+ lines changed, but ~300 of those are just adding/refactoring automated tests!!!

We already had get_prompt_parameters, but this isn't good enough if parameters aren't defined locally and we want to bubble up to the aiconfig defaults. See comment in #661 (comment).

This is important so that we can ensure we have valid params if not defined in the correct section of our AIConfig schema. This is similar in theme to #600

The only callsite we have today for this is in python in the params.py function, so I updated that callsite too. I don't know why it doesn't exist in Typescript (cc @rholinshead) but I also added this functionality in typescript in #669 (I'm not as familiar with automated testing there so will ask Ryan for a code pointer to help get me started)

Test plan

Added a bunch of automated tests which you can run by doing going to the aiconfig/python dir and running pytest

Stack created with Sapling. Best reviewed with ReviewStack.

rholinshead · 2023-12-29T18:10:12Z

python/src/aiconfig/schema.py

+
+ assert prompt is None or isinstance(prompt, Prompt)
+ if prompt is None or not prompt.metadata or not prompt.metadata.parameters:
+ return self.get_global_parameters()


It's confusing to return the global parameters when the requested prompt doesn't have any parameters, but otherwise returning only the prompt parameters if they exist.

In the parameter resolution we actually use a union of global parameters & local parameters (overriding global if they match). Shouldn't this do the same?

It's confusing to return the global parameters when the requested prompt doesn't have any parameters, but otherwise returning only the prompt parameters if they exist.

If the prompt parameters do not exist, it makes sense to check if any are defined in the global params. It's the same concept as #600

In the parameter resolution we actually use a union of global parameters & local parameters (overriding global if they match)

Tbh I feel like "check the value closest in scope (prompt) and then check the next closest value in scope (aiconfig)" is more intuitive because it's something a real human would do, but yea it doesn't matter, I can also implement it this way

Rename to resolve_prameters or something?

Maybe get_available_parameters or similar? That avoids the potential for confusion about which parameters this refers to; resolve_parameters to me means to resolve the parameters that are being used within the prompt input.

And I think this should do the same logic as in resolve_prompt_string (less reference/input params) which:

gets config-level params first

gets current prompt params, overwriting matching config-level param names with prompt-level value

Otherwise it's pretty confusing to have different behaviours and removes most of the benefits you've listed about reusability / testability.

And I think this should do the same logic as in resolve_prompt_string
gets config-level params first
gets current prompt params, overwriting matching config-level param names with prompt-level value

Yea sure, I can do that as well as change the name to get_available_parameters! I will fix forward to avoid merge conflicts: #695

rholinshead · 2023-12-29T18:14:37Z

We already had get_prompt_parameters, but this isn't good enough if parameters aren't defined locally and we want to bubble up to the aiconfig defaults. See comment in #661 (comment).

More generally, is this change actually needed? The bubbling up already happens when running the prompt, right? So the editor implementation doesn't need to explicitly get the local + global params because it will be run with the params set

rossdanlm · 2023-12-29T20:57:50Z

More generally, is this change actually needed? The bubbling up already happens when running the prompt, right? So the editor implementation doesn't need to explicitly get the local + global params because it will be run with the params set

You don't need to pass in params anymore from the client, this abstracts it away. You still can pass it in explicitly into the run() command as a method arg if you want, but this simplifies it a lot

The bubbling up already happens when running the prompt, right?

This is also done INSIDE of the run() implementation, which means the logic is tightly coupled to it. This way we have separated out the logic into it's own functionality making it:

Easier to re-use --> Can be used anywhere as a separate unit of functionality
Easier to test --> Don't need to test the entire run-flow, can be unit tested much more easily (like what I already did this diff)
Easier to extend --> Ex: if we ever want to extend this functionality to be get_XYZ, not just get_parameters (similar to issue fixed by [issue #454][AIC-py] get default model if no prompt model #600 for getting the default models), this is much easier to do now since we only need to change a method, not touch anything that could affect or break the entire run implementation

@rholinshead

**Disclaimer:** I know this is 400+ lines changed, but ~300 of those are just adding/refactoring automated tests!!! We already had `get_prompt_parameters`, but this isn't good enough if parameters aren't defined locally and we want to bubble up to the aiconfig defaults. See comment in #661 (comment). This is important so that we can ensure we have valid params if not defined in the correct section of our AIConfig schema. This is similar in theme to #600 The only callsite we have today for this is in python in the `params.py` function, so I updated that callsite too. I don't know why it doesn't exist in Typescript (cc @rholinshead) but I also added this functionality in typescript in #669 (I'm not as familiar with automated testing there so will ask Ryan for a code pointer to help get me started) ## Test plan Added a bunch of automated tests which you can run by doing going to the `aiconfig/python` dir and running `pytest`

jonathanlastmileai · 2024-01-02T14:48:31Z

python/src/aiconfig/schema.py

+ or default_return_value
+ # pylint: enable=W0102
+
+ def _get_prompt_parameters_exact(


Just FYI, {} is falsey in expressions like self._get_global_parameters_exact() or default_return_value, just like None is.

jonathanlastmileai · 2024-01-02T14:49:34Z

python/src/aiconfig/schema.py

+
+ assert prompt is None or isinstance(prompt, Prompt)
+ if prompt is None or not prompt.metadata or not prompt.metadata.parameters:
+ return self.get_global_parameters()


Rename to resolve_prameters or something?

jonathanlastmileai · 2024-01-02T16:22:05Z

ac to unblock

Create update_model endpoint TSIA, backend implementation of `update_model()`. The other changes were from running the auto-formatter: ``` fd --glob '*.py' python/src/aiconfig/editor/server | xargs python -m 'scripts.lint' --mode=fix --files ``` ## Test Plan Follow dev README to setup the local editor: https://github.com/lastmile-ai/aiconfig/tree/main/python/src/aiconfig/editor#dev, then run this command ``` curl http://localhost:8080/api/update_model -d curl http://localhost:8080/api/update_model -d '{"prompt_name":"get_activities", "model_name": "gpt-4", "settings": {"top_p": 0.9}}' -X POST -H 'Content-Type: application/json' ``` Notice that the model name is updated to `gpt-4`, and the `top_p` is now at 0.9 https://github.com/lastmile-ai/aiconfig/assets/151060367/3031fa59-8925-495c-a5eb-e56ec65b7fba --- Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/688). * #693 * #692 * #691 * #690 * __->__ #688 * #670 * #668

Create delete_parameter endpoint TSIA ## Test Plan Follow dev README to setup the local editor: https://github.com/lastmile-ai/aiconfig/tree/main/python/src/aiconfig/editor#dev, then run this command ``` curl http://localhost:8080/api/delete_parameter -d '{"prompt_name":"get_activities", "parameter_name": "city"}' -X POST -H 'Content-Type: application/json' ``` Notice that the parameters are now deleted Prompt-level https://github.com/lastmile-ai/aiconfig/assets/151060367/866db51d-6b84-438d-ac4e-483d0cfbaa40 AIConfig-level https://github.com/lastmile-ai/aiconfig/assets/151060367/dcd47f62-875c-40ac-9e81-efcd8cf9b45a --- Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/691). * #693 * #692 * __->__ #691 * #690 * #688 * #670 * #668

Create set_name endpoint TSIA ## Test Plan Follow dev README to setup the local editor: https://github.com/lastmile-ai/aiconfig/tree/main/python/src/aiconfig/editor#dev, then run this command ``` curl http://localhost:8080/api/set_name -d '{"name":"cool_new_name"}' -X POST -H 'Content-Type: application/json' ``` https://github.com/lastmile-ai/aiconfig/assets/151060367/efcb7ceb-e390-4208-93b3-3effd5d66f8b --- Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/692). * #693 * __->__ #692 * #691 * #690 * #688 * #670 * #668

Create set_description endpoint TSIA ## Test Plan Follow dev README to setup the local editor: https://github.com/lastmile-ai/aiconfig/tree/main/python/src/aiconfig/editor#dev, then run this command ``` curl http://localhost:8080/api/set_description -d '{"description":"yo this is a cool new description get rekt dont @ me bro"}' -X POST -H 'Content-Type: application/json' ``` https://github.com/lastmile-ai/aiconfig/assets/151060367/0db278b3-8273-4178-8010-380538dcd0ca --- Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/693). * __->__ #693 * #692 * #691 * #690 * #688 * #670 * #668

rossdanlm force-pushed the pr668 branch from 624d6e5 to a1a5d93 Compare December 29, 2023 04:46

rossdanlm changed the title ~~Created get_parameters API~~ [python] Created get_parameters API Dec 29, 2023

This was referenced Dec 29, 2023

[python] Updated set_parameter() to work if parameters haven't been defined before #670

Merged

[typescript] Created get_parameters API #669

Draft

rossdanlm force-pushed the pr668 branch 3 times, most recently from ed724cf to 2d20bb9 Compare December 29, 2023 05:33

rossdanlm marked this pull request as ready for review December 29, 2023 05:33

rossdanlm requested review from saqadri, rholinshead, suyoglastmileai, Ankush-lastmile and jonathanlastmileai as code owners December 29, 2023 05:33

This was referenced Dec 29, 2023

Refactor aiconfig.run() to not take in params method argument #671

Closed

[editor][server endpoints][2/n]: Add params arg to run command and check for undefined prompt_name #613

Merged

rossdanlm force-pushed the pr668 branch 2 times, most recently from 7c69320 to c572942 Compare December 29, 2023 05:51

rholinshead reviewed Dec 29, 2023

View reviewed changes

rossdanlm force-pushed the pr668 branch from c572942 to 5a75379 Compare January 2, 2024 03:03

This was referenced Jan 2, 2024

Create update_model endpoint #688

Merged

Create set_parameter endpoint #690

Merged

Create delete_parameter endpoint #691

Merged

Create set_name endpoint #692

Merged

Create set_description endpoint #693

Merged

jonathanlastmileai reviewed Jan 2, 2024

View reviewed changes

jonathanlastmileai approved these changes Jan 2, 2024

View reviewed changes

rossdanlm mentioned this pull request Jan 2, 2024

Followup fixes to get_parameters refactor PR #695

Open

rossdanlm merged commit 6bb4f7c into main Jan 2, 2024
3 checks passed

rossdanlm deleted the pr668 branch January 2, 2024 16:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[python] Created `get_parameters` API #668

[python] Created `get_parameters` API #668

rossdanlm commented Dec 29, 2023 •

edited

Loading

rholinshead Dec 29, 2023

rossdanlm Dec 29, 2023

jonathanlastmileai Jan 2, 2024

rholinshead Jan 2, 2024

rossdanlm Jan 2, 2024

rholinshead commented Dec 29, 2023

rossdanlm commented Dec 29, 2023 •

edited

Loading

jonathanlastmileai Jan 2, 2024

jonathanlastmileai Jan 2, 2024

jonathanlastmileai commented Jan 2, 2024

[python] Created get_parameters API #668

[python] Created get_parameters API #668

Conversation

rossdanlm commented Dec 29, 2023 • edited Loading

Test plan

rholinshead Dec 29, 2023

Choose a reason for hiding this comment

rossdanlm Dec 29, 2023

Choose a reason for hiding this comment

jonathanlastmileai Jan 2, 2024

Choose a reason for hiding this comment

rholinshead Jan 2, 2024

Choose a reason for hiding this comment

rossdanlm Jan 2, 2024

Choose a reason for hiding this comment

rholinshead commented Dec 29, 2023

rossdanlm commented Dec 29, 2023 • edited Loading

jonathanlastmileai Jan 2, 2024

Choose a reason for hiding this comment

jonathanlastmileai Jan 2, 2024

Choose a reason for hiding this comment

jonathanlastmileai commented Jan 2, 2024

[python] Created `get_parameters` API #668

[python] Created `get_parameters` API #668

rossdanlm commented Dec 29, 2023 •

edited

Loading

rossdanlm commented Dec 29, 2023 •

edited

Loading