Neurips client

[drisspg]

To test against local server exposed on 8080

echo 'entries: [{description: "mmlu:subject=philosophy,model=neurips/local", priority: 1}]' > run_specs.conf

helm-run --conf-paths run_specs.conf --suite v1 --max-eval-instances 1

[RaulPPelaez]

Is this timeout a constraint for the challenge?

[HarounH]

@drisspg can we increase the default timeout? 10 seconds is too small for: toy-submission but using 7B LLaMA2, on 4090, for GSM scenario. I believe that is within the specs of the competition you're running? :)

[drisspg]

Hey @yifanmai do you think it would be worthwhile to merge something like this in to the main repo? I can re name to http client and rebase

cc @msaroufim

[yifanmai]

I think this is generally good, but I would like to get #1761 in first to resolve #1673 first (so that we don't have a situation where everyone's model is named identically...)

[yifanmai]

I think this PR can be merged soon.

After merging this, I'd want to move to something like what #1761 is doing (see the PR description for an example) where a user would specify model_deployments.yaml and then run helm-run --model-deployments-paths model_deployments.yaml ...

This would ensure that each submitter's model is named something unique when you combine the benchmark_output files from all submitters, rather than having everything be named neurips/local.

I can send you a follow-up PR next week that does this early next week. I think the things that need to happen are:

1. Support instantiating clients from model_deployments.yaml that don't need an API key (like this one).
2. Support configurable window service / tokenizers.

Example: model_deployments.yaml

model_deployments:
  - name: efficiencychallenge/model-1
    model_name: efficiencychallenge/model-1
    tokenizer_name: "huggingface/gpt2"
    max_sequence_length: 2048
    client_spec:
      class_name: "helm.proxy.clients.http_client.HTTPClient"
      args:
        url: "http://localhost:8000/"
        url: "http://localhost:8000/"
        do_cache: false

[yifanmai]

If we're keeping this, then rename the class (see the comments on HTTPClient later)

[yifanmai]

Thinking about this a bit more: if all of the models listed in the rules have a Hugging Face tokenizer, then we can just use HuggingFaceWindowService for all of them, and not need a new window service.

In fact, we can even delete the tokenize() method on the client later, because we just do tokenization using the local Hugging Face tokenizer.

[drisspg]

Do you think though that since this could be more generic than just the comp it could still be useful to have a HTTPModelWindowServce?

[yifanmai]

Maybe... this is basically a generic LocalWindowService subclass and doesn't do anything HTTP specific. I think we can keep this in this PR, and when we get window service configuration in, this class will probably disappear in that refactor.

[yifanmai]

Do the special tokens need to be user-configurable?

[drisspg]

Yes I would imagine so, not sure what the best way of specifying that would be

[yifanmai]

One idea is to introduce tokenizers.yaml:

tokenizers.yaml

tokenizers:
  - name: "efficiencychallenge/tokenizer-1"
    client_spec:
      class_name: "helm.proxy.clients.http_client.HTTPClient"
      args:
        url: "http://localhost:8000/"
        prefix_token: "<|endoftext|>"
        end_of_text_token: "<|endoftext|>"

model_deployments.yaml

model_deployments:
  - name: efficiencychallenge/model-1
    model_name: efficiencychallenge/model-1
    tokenizer_name: "efficiencychallenge/tokenizer-1"
    max_sequence_length: 2048
    client_spec:
      class_name: "helm.proxy.clients.http_client.HTTPClient"
      args:
        url: "http://localhost:8000/"
        do_cache: false

cc @percyliang any thoughts on configurable tokenizers?

[percyliang]

I like configurable tokenizers.

[yifanmai]

Cool, I'll draft a PR for that.

In the meantime, I don't think we need to block this PR; we can keep these changes and clean things up later when we have configurable tokenizers.

[yifanmai]

Could you mark this as "Ready for review" (by clicking the button) when it's ready for me to take another look? Thanks!

[drisspg]

@yifanmai Cool I think I addressed everything but the yaml changes which I do think would make this service much more generic and alot more applicable to different situations

[yifanmai]

Looks good, thanks!

[yifanmai]

Sorry, I just caught a few minor issues.