Skip to content

Time individual puzzles #45

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

Closed
MLNW opened this issue Dec 5, 2023 · 10 comments · Fixed by #53
Closed

Time individual puzzles #45

MLNW opened this issue Dec 5, 2023 · 10 comments · Fixed by #53
Assignees
Labels
enhancement New feature or request

Comments

@MLNW
Copy link

MLNW commented Dec 5, 2023

In the light of solutions that may take a long time to run it would be great to have the option to time only one puzzle.
I.e.: allow cargo time 5 to only time day 5.

I guess the hardest bit would be to include the result in the benchmark table. Though it should be manageable.

@fspoettel
Copy link
Owner

fspoettel commented Dec 5, 2023

I expected this issue after today 😆. I would accept a PR for this. A cargo time --only-missing flag could also be a valid approach.

I think the complexity lies in updating the benchmark table when days or parts need to be spliced in etc.


In the meantime, maybe adjusting the benchmark count for abnormally long cases could solve the immediate pain. It is defined here:

let bench_iterations = cmp::min(
10000,
cmp::max(
Duration::from_secs(1).as_nanos() / cmp::max(base_time.as_nanos(), 10),
10,
),
);

@fspoettel fspoettel added the enhancement New feature or request label Dec 5, 2023
@fspoettel
Copy link
Owner

fspoettel commented Dec 5, 2023

you can also check out the changes in #44 we just merged. this would allow you to skip running offending slow parts for a day temporarily.

@tguichaoua
Copy link
Contributor

I think the complexity lies in updating the benchmark table when days or parts need to be spliced in etc.

The benchmark table can be cached in a json file, so that it make it easier to update a specific day or part and then update the table in the README

@MLNW
Copy link
Author

MLNW commented Dec 7, 2023 via email

@fspoettel
Copy link
Owner

The benchmark table can be cached in a json file, so that it make it easier to update a specific day or part and then update the table in the README

Good idea! 💪 That sounds like a reasonable implementation.

@fspoettel fspoettel added the good first issue Good for newcomers label Dec 7, 2023
@fspoettel fspoettel self-assigned this Dec 8, 2023
@fspoettel fspoettel removed the good first issue Good for newcomers label Dec 8, 2023
@fspoettel
Copy link
Owner

I will take a stab at this over the next couple of days, using the JSON file based approach mentioned above. This will involve creating a separate time command and extracting the logic in all::handle() to a shared helper module.

@MLNW @tguichaoua do you have any pref in regards to the default behavior of time?

Should it:
a. update only new solutions by default and have option cargo time --all to update everything?
b. update all by default and have option cargo time --only-missing to time only new solutions?

@tguichaoua
Copy link
Contributor

I prefer option A, it's more likely that the user wants to update the time for new solutions rather than recalculating everything.
Also it would be interesting to have cargo time <day> to update a specific day.

@fspoettel
Copy link
Owner

Thanks! And I agree about day, that option seems useful going back and optimizing (which I still need to do for day 5...)

@MLNW
Copy link
Author

MLNW commented Dec 8, 2023

I agree, only run time for missing days and provide the option to run time for a specific day. Thanks for taking the time!

@fspoettel
Copy link
Owner

I have a working implementation here. I settled on cargo time, cargo time --force and cargo time <day> as API.

The only limitation I can think of right now is that it's not possible to re-time a single part, mainly because it would be a large refactor and I'm not sure it's all too relevant.

I will add some much needed tests and clean it up, then open a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants