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

Request snapshot number parameter in evaluate_network() as alternative to snapshotindex. #2356

Open
Tetra-quark opened this issue Aug 17, 2023 · 4 comments · May be fixed by #2508
Open

Request snapshot number parameter in evaluate_network() as alternative to snapshotindex. #2356

Tetra-quark opened this issue Aug 17, 2023 · 4 comments · May be fixed by #2508
Assignees
Labels

Comments

@Tetra-quark
Copy link
Contributor

Tetra-quark commented Aug 17, 2023

I want to evaluate_network() on two shuffles that have a different number of snapshots in their train folders. The differing number of snapshots saved is a result of exceptions or time-outs while training on colab. Both shuffles have a particular snapshot in common that I would like to use to compare the two. It is not currently intuitive to do this.

I could run evaluate_network() for each shuffle individually specifying the snapshotindex in the config file but doing this is not especially intuitive as one has to determine what the snapshotindex actually is. I wasn't able to find an explanation in the DLC documentation although from what I can see in the repo it can be determined by listing the snapshots in the order of increasing iterations and determining the index corresponding the desired snapshot. I believe there is a simpler alternative to this.

A parameter explicitly specifying the exact snapshot number (eg. 'snapshot-120000') in the evaluate_network() function would simplify this greatly.

Alternatively adding documentation clarifying how the snapshotindex is defined (beyond the values of '-1' and 'all') could be helpful.

@MMathisLab MMathisLab added Feature-request documentation documentation updates/comments labels Aug 20, 2023
@Tetra-quark
Copy link
Contributor Author

If this feature is of any interest to DLC I have a branch in my fork that enables passing a list of snapshot names using a snapshots_to_evaluate parameter to evaluate_network() and override the snapshotindex parameter from the cfg. The default functionality still remains if the parameter is not used. Let me know!

I find it helpful to be able to pass specific, named snapshots to the evaluate_network rather than "all" or specific indices. Snapshot indices are not necessarily constant in which snapshot they refer to since the snapshot files can be created and deleted.

@jeylau
Copy link
Member

jeylau commented Dec 8, 2023

Hi @Tetra-quark, I like that feature a lot (especially since snapshots are sorted by the number of iterations rather than timestamps, which is sometimes problematic when finetuning). I'd encourage you to open a PR and document the feature, so we can review it. 😊

@hummuscience
Copy link

@Tetra-quark @jeylau was this ever introduced into DLC or is it still pending?

@Tetra-quark
Copy link
Contributor Author

@hummuscience Thanks for the nudge, I'll have a look at making a pull request today.

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

Successfully merging a pull request may close this issue.

5 participants