gpt4all icon indicating copy to clipboard operation
gpt4all copied to clipboard

Allow PrintStream as streaming source in addition to System.out.

Open raulraja opened this issue 1 year ago • 5 comments

This PR proposes enhancements to the LLModel class in the gpt4all-bindings Java project. The changes enable the use of PrintStream as an alternative to System.out for generating and streaming text.

The updated LLModel class now includes the following reviewed methods:

  1. generate(String prompt, GenerationConfig generationConfig, boolean streamToStdOut): This method generates text after the prompt, using the specified generation configuration. The streamToStdOut parameter determines whether the generated text should be streamed to the standard output. If set to true, the text is printed to System.out. Same as before.

  2. generate(String prompt, GenerationConfig generationConfig, PrintStream printStream): This method generates text after the prompt, using the specified generation configuration. The printStream parameter allows you to provide a custom PrintStream object where the generated text will be printed. This is useful for troubleshooting and redirecting the output to a different stream.

  3. generate(String prompt, GenerationConfig generationConfig, PrintStream stream, boolean enableStreaming): This method generates text after the prompt, using the specified generation configuration. The stream parameter allows you to provide a custom PrintStream object where the generated text will be printed. The enableStreaming parameter determines whether the text streaming functionality is enabled. If set to true, the generated text is streamed to the specified PrintStream.

These new methods provide flexibility in handling the generated text output, allowing developers to choose between the standard output (System.out) and custom PrintStream objects. They can be utilized for troubleshooting, redirecting output, or integrating with other logging mechanisms.

Additionally, this PR includes an example (Example5.java) that demonstrates the usage of the new generate methods. It showcases how to generate text using the LLModel class and stream the output to a custom PrintStream.

raulraja avatar Jul 03 '23 15:07 raulraja

Looks good but let me test it out shortly and make a release. @raulraja

felix-zaslavskiy avatar Jul 24 '23 22:07 felix-zaslavskiy

@cosmic-snow This PR looks good. Could we have it merged? I will have the change included in the next release of Java binding wich will be 1.1.6

felix-zaslavskiy avatar Jul 25 '23 19:07 felix-zaslavskiy

Questions:

  • Do I need to check it out and test myself? I had a look at the code and it looked fine, but that doesn't say much, considering my Java expertise these days. 😅
  • The only thing I noticed was that docstring there. Are you going to fix it yourself?
  • Oh also, looks like it needs to be rebased/merged first? GitHub says it has conflicts.

cosmic-snow avatar Jul 25 '23 20:07 cosmic-snow

Questions:

  • Do I need to check it out and test myself? I had a look at the code and it looked fine, but that doesn't say much, considering my Java expertise these days. 😅
  • The only thing I noticed was that docstring there. Are you going to fix it yourself?
  • Oh also, looks like it needs to be rebased/merged first? GitHub says it has conflicts.

@raulraja Please rebase on the master branch and the Javadoc fix. thanks

felix-zaslavskiy avatar Jul 25 '23 23:07 felix-zaslavskiy

@raulraja @cosmic-snow @felix-zaslavskiy I've rebased the changes from this PR onto the current main branch, the rebased commit can be found at https://github.com/floscher/gpt4all/commit/82d32dcf65b38d05bba52ee2451354f26f693c69 on this branch of my fork: https://github.com/floscher/gpt4all/tree/streaming-callbacks

The changes to the source files look good to me. The tests might need some amendments, e.g. the rebased commit re-adds the parts of the Tests.java file which this PR touches, but on the main branch the file was deleted with 4e274baee154eb65b02257733e927445442baf3b.

floscher avatar Dec 19 '23 16:12 floscher

Is this out of date and to be closed?

manyoso avatar Mar 06 '24 19:03 manyoso

Is this out of date and to be closed?

The Java bindings are completely unmaintained right now (they do not work as of current master), so this PR is not currently relevant.

cebtenzzre avatar Mar 06 '24 19:03 cebtenzzre

Is this out of date and to be closed?

The Java bindings are completely unmaintained right now (they do not work as of current master), so this PR is not currently relevant.

Very unfortunate. I suggest we put out a call to update the Java bindings, but if no one steps up they should be removed :(

manyoso avatar Mar 09 '24 15:03 manyoso

@floscher Just FYI and sorry for the late reply: Felix was originally maintaining the Java bindings, I was merely helping out with some things. But even a PR I made to help him with his work (#1348) was rejected. So I'm definitely not the right person to ping here.

I don't know if Felix has ever contributed anything again after that point — I'm kind of not up-to-date on things, either — so you'll have to direct anything else at manyoso and/or cebtenzzre, I think.

cosmic-snow avatar Mar 12 '24 06:03 cosmic-snow