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

LogCombiner 2.7.7: confusing naming of resampling input field #84

Open
tgvaughan opened this issue Jul 10, 2024 · 4 comments
Open

LogCombiner 2.7.7: confusing naming of resampling input field #84

tgvaughan opened this issue Jul 10, 2024 · 4 comments

Comments

@tgvaughan
Copy link
Contributor

Hi Remco,

When instructing someone on the use of LogCombiner, I noticed that the resampling input field is confusingly labeled "Resample states at lower frequency".

From the label it's not clear exactly what LC is expecting here. By "lower frequency" it sounds like this is expecting a frequency, i.e. a the number of samples produced per iteration - with lower values indicating sparser sampling. But it's actually expecting the period between samples, as we know because we use it.

Would it be possible to make this clearer, at least in the GUI?

(Another idea would be to have LogCombiner take something like the fraction of samples to include - or an integer frequency reduction factor - so that users don't need to remember what the original sampling period was in order to figure out what to enter in this field.)

@rbouckaert
Copy link
Member

I can confirm from personal experience that it is very confusing!

What I would rather want to specify is the number of trees that are present in the file to skip, so something like logcombiner -skip 5 to get 20% of the original tree set -- pretty much what you already suggested.

Perhaps the best way forward is to have a new option -skip that must be enforced to be mutually exclusive from using the -resample option for the CLI version.

For the GUI version, we can have a drop-down box containing both options and one field to edit a numeric value for the chosen option.

Does that sound reasonable/any suggestions?

@tgvaughan
Copy link
Contributor Author

Hi Remco, this sounds perfect. Although perhaps call the option "-includeEvery"? Otherwise "-skip 5" sounds to me like it should include every 6th sample.

@rbouckaert
Copy link
Member

Good point: -includeEvery it will be.

@rbouckaert
Copy link
Member

Added a label "resample strategy" to the GUI, but now not so sure whether the default "No resampling" is confusing. Perhaps "All samples included" makes more sense.

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

No branches or pull requests

2 participants