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.

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

make it safer input argument type check in sampling method #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wbaek
Copy link
Contributor

@wbaek wbaek commented Mar 25, 2022

In my case, during T2I sampling

In the part of declaring the following variable
When using as int type like this

top_k=1024
top_p=1

I met the following error message:

object of type 'int' has no len()

@chiheonk
Copy link
Contributor

This is indeed better and LGTM, but this will break the scripts when they are parsing topk, topp from config, in particular:

if args.top_k is None:
args.top_k = config.sampling.top_k
if args.top_p is None:
args.top_p = config.sampling.top_p

if args.top_k is None:
args.top_k = config.sampling.top_k
if args.top_p is None:
args.top_p = config.sampling.top_p

This is due to that args.topk, args.topp is to be omegaconf.ListConfig and when passed to the method sample, type checking will cause error.
So we would need a fix in those scripts to convert ListConfig into list or tuple.

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