search

BEAST2-Dev / bdsky

Birth Death Serial Skyline Model for BEAST2
GNU General Public License v3.0
2 stars 11 forks source link

Not using SA operators for SA analysis should be a fatal error #23

Closed laduplessis closed 4 years ago

laduplessis commented 6 years ago

Current behaviour when running a sampled ancestors analysis without using sampled ancestor tree operators is to print an error message stating the analysis is not valid and continue running. The error message is printed in between other initialisation output and will probably not be noticed by many users. If the analysis is not valid the error should be fatal and the program should refuse to run.

tgvaughan commented 4 years ago

Better late than never! The commit looks sensible; after all, what's the point of continuing with an analysis after a warning like "this analysis is not valid"?

Just a comment on the checking code itself though - it looks very fragile. This check could conclude that the operators are unsuitable even when they are if SA-specific operators extended any of the operators in the hard-coded list. Conversely, the check could conclude that the operators are suitable when they're not, if non SA-specific tree operators are being used which are not on that list.

But I suppose the intent here is to catch the majority of the problems - people writing their own tree operators are capable of submitting pull requests. :-)