Refactor to put 'Lite' functionality into ExtractCohort. #8295

gbggrant · 2023-04-24T19:11:20Z

No description provided.

codecov · 2023-04-24T19:42:03Z

Codecov Report

❗ No coverage uploaded for pull request base (ah_var_store@0b4e305). Click here to learn what that means.
The diff coverage is n/a.

Additional details and impacted files

@@               Coverage Diff                @@
##             ah_var_store     #8295   +/-   ##
================================================
  Coverage                ?   86.188%           
  Complexity              ?     35524           
================================================
  Files                   ?      2192           
  Lines                   ?    166470           
  Branches                ?     17917           
================================================
  Hits                    ?    143478           
  Misses                  ?     16606           
  Partials                ?      6386

gatk-bot · 2023-04-27T21:04:05Z

Github actions tests reported job failures from actions build 4824196683
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	11	4824196683.13	logs
unit	8	4824196683.1	logs

gatk-bot · 2023-04-28T19:32:33Z

Github actions tests reported job failures from actions build 4833917205
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	11	4833917205.13	logs
unit	8	4833917205.1	logs

…geExtractCohortLiteIntoExtractCohort

gatk-bot · 2023-05-03T18:34:52Z

Github actions tests reported job failures from actions build 4875101629
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	11	4875101629.13	logs
unit	8	4875101629.1	logs

gatk-bot · 2023-05-03T18:35:10Z

Github actions tests reported job failures from actions build 4875090542
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	11	4875090542.13	logs
unit	8	4875090542.1	logs

gatk-bot · 2023-05-03T19:40:37Z

Github actions tests reported job failures from actions build 4875589595
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	11	4875589595.13	logs
unit	8	4875589595.1	logs

gatk-bot · 2023-05-03T20:24:56Z

Github actions tests reported job failures from actions build 4875986159
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	11	4875986159.13	logs
integration	11	4875986159.12	logs

gbggrant · 2023-05-05T01:05:58Z

Passing VQSR Lite Run Here
Passing VQSR Classic Run Here.

mcovarr

Looks good despite my "Stream" of comments 😛

mcovarr · 2023-05-05T20:18:39Z

src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/ExtractCohort.java

+            doc = "If true, use VQSR 'Classic' scoring (vqs Lod score). Otherwise use VQSR 'Lite' scoring (calibration_sensitivity) ",
+            optional = true
+    )
+    private boolean isVQSRClassic = true;


The way I'm reading the doc it sounds like the default should be Lite, but actually the default is Classic.

mcovarr · 2023-05-05T20:21:05Z

src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/ExtractCohort.java

@@ -143,28 +118,33 @@ public enum VQSLODFilteringType { GENOTYPE, SITES, NONE }
    )
    private boolean emitADs = false;

-    // what if this was a flag input only?


I am also wondering this

src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/ExtractCohortEngine.java

mcovarr · 2023-05-05T21:07:08Z

src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/ExtractCohortLite.java

+            doc = "If true, use VQSR 'Classic' scoring (vqs Lod score). Otherwise use VQSR 'Lite' scoring (calibration_sensitivity) ",
+            optional = true
+    )
+    private boolean isVQSRClassic = true;


same comment here as before

…ExtractCohortEngine.java Co-authored-by: Miguel Covarrubias <[email protected]>

…geExtractCohortLiteIntoExtractCohort

gatk-bot · 2023-05-09T15:34:50Z

Github actions tests reported job failures from actions build 4927370223
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	11	4927370223.13	logs
unit	8	4927370223.1	logs

src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/ExtractCohortEngine.java

RoriCremer · 2023-05-09T16:23:12Z

src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/ExtractCohortEngine.java

+                .filter(d -> !(d.isNaN()||d.isInfinite()))
+                .max(Double::compareTo);
+        return maxVal.isPresent() && maxVal.get() < vqScoreThreshold;
+    }


not necessary for this pr, but I think additional documentation around this is necessary.
also lol do we really get vqscores of "infinite"?

Yeah, that's a good point. Not sure if we'd ever see infinite, but am hesitant to pull that check out.

src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/ExtractCohortEngine.java

RoriCremer · 2023-05-09T16:31:37Z

src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/ExtractCohortEngine.java

-                throw new UserException("Vqslod filtering thresholds for SNPs and INDELs must be defined.");
+        boolean noVQScoreFilteringRequested = vqScoreFilteringType.equals(ExtractCohort.VQScoreFilteringType.NONE);
+        if (!noVQScoreFilteringRequested) {
+            // ensure vqScore filters (vqsLod or Sensitivity) are defined. this really shouldn't ever happen, said the engineer.


dont we have defaults?

Yeah, we do. I didn't write that comment, but I can dig a little deeper to see if it could.

RoriCremer

A few nits that I'm happy to be talked out of

gatk-bot · 2023-05-09T19:25:34Z

Github actions tests reported job failures from actions build 4929394589
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	11	4929394589.13	logs
unit	8	4929394589.1	logs

gbggrant · 2023-05-09T19:46:10Z

Passing VQSR Lite Run Here
Passing VQSR Classic Run Here.

gbggrant · 2023-05-10T15:00:48Z

retest this please

Starting refactoring of ExtractCohortLite into ExtractCohort.

7fe2be8

gbggrant added 6 commits April 24, 2023 16:28

More refactoring

49414ca

More refactoring

780114e

More refactoring - unified a test.

f87f148

Gotta annotate a test as a test

1b9afce

More refactoring

ffe32a3

More refactoring

d2ca671

Almost there - still need to flip the sign in the engines.

781e2c5

gbggrant added 2 commits May 3, 2023 14:05

Fix arg parser error.

962ccef

Merge remote-tracking branch 'origin/ah_var_store' into gg_VS-892_Mer…

31b5be2

…geExtractCohortLiteIntoExtractCohort

Create new overrideable method to determine failing sites/genotypes.

1cd271b

gbggrant added 2 commits May 3, 2023 15:56

Ready for testing?

4a58411

Cleaned up a mutex.

c6d2c7d

Pass argument differently.

8ae98fb

gbggrant changed the title ~~Starting refactoring of ExtractCohortLite into ExtractCohort.~~ Refactor to put 'Lite' functionality into ExtractCohort. May 4, 2023

gbggrant marked this pull request as ready for review May 5, 2023 01:02

gbggrant requested review from mcovarr and RoriCremer May 5, 2023 01:02

gbggrant added 3 commits May 5, 2023 13:36

Update the docker for the Extract task.

3a7d181

Fix argument initialization error for VQSR Classic mode.

d0ba72a

Update the docker

ae2afa0

mcovarr reviewed May 5, 2023

View reviewed changes

gbggrant and others added 13 commits May 8, 2023 10:10

Update src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/…

3e2b5db

…ExtractCohortEngine.java Co-authored-by: Miguel Covarrubias <[email protected]>

Update src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/…

795835e

…ExtractCohortEngine.java Co-authored-by: Miguel Covarrubias <[email protected]>

Update src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/…

9085f3b

…ExtractCohortEngine.java Co-authored-by: Miguel Covarrubias <[email protected]>

Update src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/…

de54230

…ExtractCohortEngine.java Co-authored-by: Miguel Covarrubias <[email protected]>

Update src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/…

26455e6

…ExtractCohortEngine.java Co-authored-by: Miguel Covarrubias <[email protected]>

Update src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/…

d788b14

…ExtractCohortEngine.java Co-authored-by: Miguel Covarrubias <[email protected]>

Update src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/…

0ccfbc1

…ExtractCohortEngine.java Co-authored-by: Miguel Covarrubias <[email protected]>

Merge remote-tracking branch 'origin/ah_var_store' into gg_VS-892_Mer…

f1b7f8b

…geExtractCohortLiteIntoExtractCohort

Import a class

72c8ba7

Need to change the signature of the overriding method too!

511a219

Correct the logic for VQSR Lite?

e3965f1

Update the docker

1503cd0

Make vqsr lite the default.

50d636f

RoriCremer reviewed May 9, 2023

View reviewed changes

src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/ExtractCohortEngine.java Show resolved Hide resolved

RoriCremer reviewed May 9, 2023

View reviewed changes

src/main/java/org/broadinstitute/hellbender/tools/gvs/extract/ExtractCohortEngine.java Show resolved Hide resolved

RoriCremer reviewed May 9, 2023

View reviewed changes

RoriCremer approved these changes May 9, 2023

View reviewed changes

Updated the docker

0fa9359

Integrated ExtractCohortLiteTest.java into ExtractCohortTest.java

328ba6b

mcovarr approved these changes May 9, 2023

View reviewed changes

Additional comments.

3b8702c

gbggrant added 2 commits May 10, 2023 11:02

Additional comment to retrigger the tests.

6008a78

Additional comment to retrigger the tests.

b12d103

gbggrant merged commit 834e6da into ah_var_store May 10, 2023

gbggrant deleted the gg_VS-892_MergeExtractCohortLiteIntoExtractCohort branch May 10, 2023 18:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor to put 'Lite' functionality into ExtractCohort. #8295

Refactor to put 'Lite' functionality into ExtractCohort. #8295

gbggrant commented Apr 24, 2023

codecov bot commented Apr 24, 2023 •

edited

Loading

gatk-bot commented Apr 27, 2023 •

edited

Loading

gatk-bot commented Apr 28, 2023 •

edited

Loading

gatk-bot commented May 3, 2023 •

edited

Loading

gatk-bot commented May 3, 2023 •

edited

Loading

gatk-bot commented May 3, 2023 •

edited

Loading

gatk-bot commented May 3, 2023 •

edited

Loading

gbggrant commented May 5, 2023

mcovarr left a comment

mcovarr May 5, 2023

mcovarr May 5, 2023

mcovarr May 5, 2023

gatk-bot commented May 9, 2023 •

edited

Loading

RoriCremer May 9, 2023

gbggrant May 9, 2023

RoriCremer May 9, 2023

gbggrant May 9, 2023

RoriCremer left a comment

gatk-bot commented May 9, 2023 •

edited

Loading

gbggrant commented May 9, 2023 •

edited

Loading

gbggrant commented May 10, 2023

Refactor to put 'Lite' functionality into ExtractCohort. #8295

Refactor to put 'Lite' functionality into ExtractCohort. #8295

Conversation

gbggrant commented Apr 24, 2023

codecov bot commented Apr 24, 2023 • edited Loading

Codecov Report

gatk-bot commented Apr 27, 2023 • edited Loading

gatk-bot commented Apr 28, 2023 • edited Loading

gatk-bot commented May 3, 2023 • edited Loading

gatk-bot commented May 3, 2023 • edited Loading

gatk-bot commented May 3, 2023 • edited Loading

gatk-bot commented May 3, 2023 • edited Loading

gbggrant commented May 5, 2023

mcovarr left a comment

Choose a reason for hiding this comment

mcovarr May 5, 2023

Choose a reason for hiding this comment

mcovarr May 5, 2023

Choose a reason for hiding this comment

mcovarr May 5, 2023

Choose a reason for hiding this comment

gatk-bot commented May 9, 2023 • edited Loading

RoriCremer May 9, 2023

Choose a reason for hiding this comment

gbggrant May 9, 2023

Choose a reason for hiding this comment

RoriCremer May 9, 2023

Choose a reason for hiding this comment

gbggrant May 9, 2023

Choose a reason for hiding this comment

RoriCremer left a comment

Choose a reason for hiding this comment

gatk-bot commented May 9, 2023 • edited Loading

gbggrant commented May 9, 2023 • edited Loading

gbggrant commented May 10, 2023

codecov bot commented Apr 24, 2023 •

edited

Loading

gatk-bot commented Apr 27, 2023 •

edited

Loading

gatk-bot commented Apr 28, 2023 •

edited

Loading

gatk-bot commented May 3, 2023 •

edited

Loading

gatk-bot commented May 3, 2023 •

edited

Loading

gatk-bot commented May 3, 2023 •

edited

Loading

gatk-bot commented May 3, 2023 •

edited

Loading

gatk-bot commented May 9, 2023 •

edited

Loading

gatk-bot commented May 9, 2023 •

edited

Loading

gbggrant commented May 9, 2023 •

edited

Loading