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

Fix generate to return results in submission order #8

Merged
merged 3 commits into from
Oct 28, 2024

Conversation

algal
Copy link
Contributor

@algal algal commented Oct 25, 2024

This updates generate to return generation results in the inputs order, rather than in the order generation jobs completed.

It still submits the jobs in inputs order, so the jobs will start in inputs order.

It relies on as_completed to determine, to update the progress bar whenever a job is completed.

This does not add any timeout or error handling logic, so we are still relying on the user to build that into job definitions -- e.g., to define jobs such that they will return None after a timeout period or in the event of an error.

I have lightly tested it, by executing 50 jobs with 5 workers, and inspecting results. Let's discuss @ncoop57

generate returns generated values in the same order
as the inputs used to create them, rather than in
the order in which the generation jobs completed
@ncoop57
Copy link
Contributor

ncoop57 commented Oct 25, 2024

Oh I really like this solution!!

@ncoop57
Copy link
Contributor

ncoop57 commented Oct 25, 2024

@algal Could you please add some tests into the nb that showcase it keeps thing in order? It can be a super small test.

@algal
Copy link
Contributor Author

algal commented Oct 26, 2024

@ncoop57 I added a test which provides soft confirmation that the generate function now returns outputs in the order of inputs.

@ncoop57 ncoop57 merged commit e4f7a5d into AnswerDotAI:main Oct 28, 2024
1 check failed
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