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

ctransformers: move thread and seed parameters #3543

Merged
merged 2 commits into from
Aug 13, 2023

Conversation

cal066
Copy link
Contributor

@cal066 cal066 commented Aug 12, 2023

Make it more in line with webui's way of managing loaders.

While ctransformers can have the number of threads and seed set during the loading and generation phase, set the threads during model load and seed during generate phase to align with llama.cpp expectations.

Checklist:

Make it more in line with webui's way of managing loaders.

While ctransformers can have the number of threads and seed set
during the loading and generation phase, set the threads during
model load and seed during generate phase to align with llama.cpp
expectations.

Seed value now taken from the parameters page, where -1 is for
random.
@oobabooga
Copy link
Owner

Thank you. I confirm that the threads option works this way, but the seed seems to have no effect -- if I generate twice for the same prompt but different seeds, I get two different responses. Is that a bug in ctransformers?

@bartowski1182
Copy link
Contributor

bartowski1182 commented Aug 12, 2023

if I generate twice for the same prompt but different seeds, I get two different responses.

A typo? I presumed that was intended behaviour

@oobabooga
Copy link
Owner

Yes, I meant same seeds*

@cal066
Copy link
Contributor Author

cal066 commented Aug 13, 2023

@oobabooga in ctransformers ctransformers/llm.py:

    def sample(...)
       ...
        seed = get(seed, config.seed)

        return self.ctransformers_llm_sample(
            top_k,
            top_p, 
            temperature,
            repetition_penalty,
            last_n_tokens,
            seed,
        )

Eventually it makes its way to the LLM::Sample C++ class models/llm.h:

    if (seed < 0) {
      seed = time(nullptr);
    }
    std::mt19937 rng(seed);

From what I understand, as long as seed is -1, it should be random.
@marella Can you comment?
@oobabooga What model were you testing?

@oobabooga
Copy link
Owner

I tried falcon-7b-instruct.ggccv1.q4_0.bin from here: https://huggingface.co/TheBloke/falcon-7b-instruct-GGML/tree/main

Make sure to set a high temperature/top_k/top_p to make the replies unpredictable.

@cal066
Copy link
Contributor Author

cal066 commented Aug 13, 2023

From my testing, it seems to be working (using the same model and simple-1 Parameters):

Instruction: Roll a 20-sided dice.
Output: The result of rolling a 20-sided dice is 1.
Output: The result of rolling a 20-sided dice is 20.
Output: The result of rolling a 20-sided dice is 11.

@marella
Copy link
Contributor

marella commented Aug 13, 2023

From what I understand, as long as seed is -1, it should be random.

Yes, seed=-1 means it will use a random seed.

Is shared.settings['seed'] a constant or can be changed by user from UI? Shouldn't you read it from a user configurable param like shared.args.llama_cpp_seed?

@oobabooga
Copy link
Owner

Well noted @marella. The UI parameters are passed in the state dict. I have made this fix, so it should be good now.

Thanks @cal066 for the PR and thanks @marella for the feedback (and for ctransformers :).

@oobabooga oobabooga merged commit bf70c19 into oobabooga:main Aug 13, 2023
@@ -49,6 +47,7 @@ def decode(self, ids):

def generate(self, prompt, state, callback=None):
prompt = prompt if type(prompt) is str else prompt.decode()
# ctransformers uses -1 for random seed
generator = self.model._stream(
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use self.model() as self.model._stream() is an internal method and should not be used directly.

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.

4 participants