Can CollectWgsMetrics fall back to slow algorithm gracefully if the fast one fails with the default read length?

[gokalpcelik]

This issue seems to be present in the latest version as well. I have a collection of WGS bam files (#>100) and fast algorithm works gracefully for most of them whereas only a few of them fail with the same error due to read length is not being optimal

java.lang.ArrayIndexOutOfBoundsException: The requested index 1247601 is out of counter bounds. Possible cause of exception can be wrong READ_LENGTH parameter (much smaller than actual read length)
	at picard.analysis.CounterManager$Counter.checkIndex(CounterManager.java:189)
	at picard.analysis.CounterManager$Counter.get(CounterManager.java:183)
	at picard.analysis.FastWgsMetricsCollector.processRecord(FastWgsMetricsCollector.java:137)
	at picard.analysis.FastWgsMetricsCollector.addInfo(FastWgsMetricsCollector.java:105)
	at picard.analysis.WgsMetricsProcessorImpl.processFile(WgsMetricsProcessorImpl.java:93)
	at picard.analysis.CollectWgsMetrics.doWork(CollectWgsMetrics.java:242)
	at picard.cmdline.CommandLineProgram.instanceMain(CommandLineProgram.java:280)
	at org.broadinstitute.hellbender.cmdline.PicardCommandLineProgramExecutor.instanceMain(PicardCommandLineProgramExecutor.java:37)
	at org.broadinstitute.hellbender.Main.runCommandLineProgram(Main.java:166)
	at org.broadinstitute.hellbender.Main.mainEntry(Main.java:209)
	at org.broadinstitute.hellbender.Main.main(Main.java:306)

I can understand that this parameter is important for estimates however instead of failing can the tool fall back to the slow algorithm gracefully?

[lbergelson]

@gokalpcelik What are you envisioning exactly? If it encounters an error it will restart at the beginning of the bam with the slow algorithm? Or it will process a specific read with the other algorithm and then combines them?

[gokalpcelik]

This usually happens at the very beginning so restarting the traversal should not be a huge loss.

[lbergelson]

That seems reasonable then.

[lbergelson]

I wonder if it's possible to make the Fast collector work on these files?

If it's just retrying with the other collector that seems pretty simple, but kind of gross. Do you want to implement that? I might change the arguments so instead of USE_FAST_ALGORITHM we have a set of inputs SLOW, FAST, TRY_FAST_AND_FALL_BACK or something like that so you can control the behavior.

[gokalpcelik]

Try fast and fall back is a nice option

…

On Tue, Jul 9, 2024, 20:36 Louis Bergelson ***@***.***> wrote: I wonder if it's possible to make the Fast collector work on these files? If it's just retrying with the other collector that seems pretty simple, but kind of gross. Do you want to implement that? I might change the arguments so instead of USE_FAST_ALGORITHM we have a set of inputs SLOW, FAST, TRY_FAST_AND_FALL_BACK or something like that so you can control the behavior. — Reply to this email directly, view it on GitHub <#1970 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AI6VAC6UCE53HGDXWCMV6ELZLQNRTAVCNFSM6AAAAABKQURVQSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJYGI4TKNJYHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[lbergelson]

We will have to identify what the actual issue that's causing this is, because we want to only fall back in the cases where it would help to do so, and not just any random ArrayIndexOutOfBoundsException . We'll probably want to change it to throw something like ReadTooLongForFastCollectorException (I don't know enough to say if that's really the issue, but whatever it is.)

[kockan]

@lbergelson It is the issue. The READ_LENGTH parameter is used by FastWgsMetricCollector to allocate arrays (wrapped by CounterManager) and if some of your reads are longer than the value of READ_LENGTH this might cause ArrayIndexOutOfBoundsException in certain cases. Unfortunately, there is no way to set this value in a smart way without performing a first pass over your alignment file and get the maximum read length, and dynamically reallocating arrays at runtime should be a no-go since it affects the processing of previous alignments (don't quote me on this, maybe this can be done somehow, I am not familiar with the intricacies of FastWgsMetricCollector).

cc: @gokalpcelik

[multimeric]

The READ_LENGTH parameter is used by FastWgsMetricCollector to allocate arrays (wrapped by CounterManager) and if some of your reads are longer than the value of READ_LENGTH this might cause ArrayIndexOutOfBoundsException in certain cases.

If this is true, then perhaps the argument is mislabelled? Currently it is described as the average read length.