Skip to content

Conversation

@achraf-mer
Copy link
Collaborator

No description provided.

@achraf-mer achraf-mer marked this pull request as draft July 27, 2023 21:36
python generate.py --base_model=h2oai/h2ogpt-oig-oasst1-512-6_9b
"""
fire.Fire(main)
H2O_Fire(main)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ChathurindaRanasinghe I only changed here, can you please review and also check other places where env vars are needed. Thanks

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

please review post merge.

@pseudotensor
Copy link
Collaborator

Cool thanks!

Copy link
Collaborator

@pseudotensor pseudotensor left a comment

Choose a reason for hiding this comment

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

Thanks. I was thinking of something related a month ago. I wanted to be able to set any parameter of main() via ENV. Thanks for this, since I think it takes care of that too.

What about just H2OGPT_x . Why need _PARAM? Also, ok to allow all lower-case too like we do in DAI? Thanks!

@achraf-mer
Copy link
Collaborator Author

achraf-mer commented Jul 27, 2023

Thanks. I was thinking of something related a month ago. I wanted to be able to set any parameter of main() via ENV. Thanks for this, since I think it takes care of that too.

What about just H2OGPT_x . Why need _PARAM? Also, ok to allow all lower-case too like we do in DAI? Thanks!

ok, done in 4b90733

no prioritization for now (i.e. all caps taking precedence) can do if needed.

also, Fire is not very lenient, for example, this works:
H2OGPT_LOAD_EXLLAMA=True or --load_exllama=True

but with lower case t it doesn't:
H2OGPT_LOAD_EXLLAMA=true or --load_exllama=true

@pseudotensor
Copy link
Collaborator

Thanks. I was thinking of something related a month ago. I wanted to be able to set any parameter of main() via ENV. Thanks for this, since I think it takes care of that too.
What about just H2OGPT_x . Why need _PARAM? Also, ok to allow all lower-case too like we do in DAI? Thanks!

ok, done in 4b90733

no prioritization for now (i.e. all caps taking precedence) can do if needed.

also, Fire is not very lenient, for example, this works: H2OGPT_LOAD_EXLLAMA=True or --load_exllama=True

but with lower case t it doesn't: H2OGPT_LOAD_EXLLAMA=True or --load_exllama=True

Ok, I think you mean 2nd case to be "true". ya, it's not toml, it's ok.

Ya, and no need for prioritization like in DAI IMO. Thanks!

@achraf-mer
Copy link
Collaborator Author

achraf-mer commented Jul 27, 2023

Ok, I think you mean 2nd case to be "true". ya, it's not toml, it's ok.

Ya, and no need for prioritization like in DAI IMO. Thanks!

oops, that's right, sorry, updated.

@pseudotensor
Copy link
Collaborator

pseudotensor commented Jul 27, 2023

Can you add a note and example here?

https://github.com/h2oai/h2ogpt/blob/main/docs/FAQ.md#what-envs-can-i-pass-to-control-h2ogpt

Thanks!

We can keep all those older envs for now, don't want to break things if others using them.

But good to know how to generally set the env for any main() parameters.

@achraf-mer
Copy link
Collaborator Author

Can you add a note and example here?

https://github.com/h2oai/h2ogpt/blob/main/docs/FAQ.md#what-envs-can-i-pass-to-control-h2ogpt

Thanks!

We can keep all those older envs for now, don't want to break things if others using them.

But good to know how to generally set the env for any main() parameters.

added here: https://github.com/h2oai/h2ogpt/blob/am/load_args_from_env/docs/FAQ.md#what-envs-can-i-pass-to-control-h2ogpt

@achraf-mer achraf-mer force-pushed the am/load_args_from_env branch from c763063 to 25fc298 Compare July 27, 2023 21:59
@achraf-mer achraf-mer force-pushed the am/load_args_from_env branch from 25fc298 to c5e040a Compare July 27, 2023 22:00
@pseudotensor pseudotensor self-requested a review July 27, 2023 22:01
@pseudotensor pseudotensor changed the title Load args from env vars as long as var starts with H2OGPT_PARAM_ Load args from env vars as long as var starts with H2OGPT_ Jul 28, 2023
@achraf-mer achraf-mer marked this pull request as ready for review July 28, 2023 16:42
@achraf-mer achraf-mer merged commit 8cf0541 into main Jul 28, 2023
@achraf-mer achraf-mer deleted the am/load_args_from_env branch July 28, 2023 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants