Update run.py #261
Update run.py #261
Conversation
| inference_runner.initialize() | ||
| except Exception as e: | ||
| logging.error(f"Failed to initialize the inference runner: {e}") | ||
| return |
There was a problem hiding this comment.
This should raise the exception as an error will cause the inference_runner to not instantiate. Returning nothing and logging the error is not sufficient enough for handling an error like this at startup.
| output = sample_from_model(gen, inp, max_len=100, temperature=0.01) | ||
| print(f"Output for prompt: '{inp}':\n{output}") | ||
| except Exception as e: | ||
| logging.error(f"Failed during model inference: {e}") |
|
|
||
| inp = "The answer to life the universe and everything is of course" | ||
| output = sample_from_model(gen, inp, max_len=100, temperature=0.01) | ||
| print(f"Output for prompt: '{inp}':\n{output}") |
There was a problem hiding this comment.
The original code does not place the output on a new line. Can you be sure this won't change behavior for clients utilizing this output for something?
There was a problem hiding this comment.
Irrelevant. You changed behavior without test coverage.
| pad_token=0, | ||
| eos_token=2, | ||
| sequence_len=8192, | ||
| sequence_len=8192, # Sequence length |
There was a problem hiding this comment.
Why add a comment that repeats the parameter name? Is there genuine concern that "sequence_len" isn't already clear?
|
This codebase is not really meant to be "beginner-friendly". This PR adds nothing of substance. |
|
|
||
| inp = "The answer to life the universe and everything is of course" | ||
| output = sample_from_model(gen, inp, max_len=100, temperature=0.01) | ||
| print(f"Output for prompt: '{inp}':\n{output}") |
There was a problem hiding this comment.
Irrelevant. You changed behavior without test coverage.
Adding comments to explain model configuration settings more clearly.
Using an f-string for the error message logging to maintain consistency.
Wrapping the model initialization and inference call in try-except blocks to catch and log exceptions, providing a clearer failure message should anything go wrong during these processes.