CompEvol / BeastFX

GUIs for BEAST using Java FX to make things more pretty.
GNU Lesser General Public License v2.1
4 stars 1 forks source link

LogCombiner 2.7.7: confusing naming of resampling input field #84

Open tgvaughan opened 2 months ago

tgvaughan commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

Good point: -includeEvery it will be.

rbouckaert commented 1 month ago

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.