Back out some changes not directly related to remote kinship execution

bbimber · bbimber · commit 2cbf5990477a · 2024-02-28T09:25:57.000-08:00
diff --git a/ehr/resources/pipeline/kinship/populateKinship.r b/ehr/resources/pipeline/kinship/populateKinship.r
@@ -13,20 +13,10 @@ library(Matrix)
 library(dplyr)
 
 spec <- matrix(c(
-    'inputFile', '-f', 1, 'character',
-    'mergeSpeciesWithHybrids', '-m', 0, 'logical',
-    'performKinshipValidation', '-v', 0, 'logical'
+    'inputFile', '-f', 1, 'character'
 ), ncol=4, byrow=TRUE)
 opts <- getopt(spec, commandArgs(trailingOnly = TRUE))
 
-if (is.null(opts$mergeSpeciesWithHybrids)){
-    opts$mergeSpeciesWithHybrids <- FALSE
-}
-
-if (is.null(opts$performKinshipValidation)){
-    opts$performKinshipValidation <- FALSE
-}
-
 allPed <- read.table(opts$inputFile, quote="\"")
 colnames(allPed)<-c('Id', 'Dam', 'Sire', 'Gender', 'Species')
 
@@ -43,147 +33,10 @@ if (any(allPed$Species == 'Unknown')) {
     print(paste0('There are ', sum(allPed$Species == 'Unknown'), ' Ids with species = Unknown'))
 }
 
-# The purpose of this function is to handle instances where there was cross-breeding.
-# While this is probably rare, it can occur. When this happens, simply merge the entire species together and process as one unit.
-# This ensures that all relevant ancestors from both species are present
-generateSpeciesToProcess <- function(allPed, mergeSpeciesWithHybrids) {
-    hybridParents <- dplyr::bind_rows(
-      merge(allPed, allPed, by.x = 'Dam', by.y = 'Id') %>% filter(Species.x != Species.y) %>% select(Id, Dam, Species.x, Species.y) %>% rename(Parent = Dam) %>% mutate(ParentType = 'Dam'),
-      merge(allPed, allPed, by.x = 'Sire', by.y = 'Id') %>% filter(Species.x != Species.y) %>% select(Id, Sire, Species.x, Species.y) %>% rename(Parent = Sire) %>% mutate(ParentType = 'Sire')
-    )
-
-    if (nrow(hybridParents) > 0) {
-        print(paste0('There were ', nrow(hybridParents), ' records with parents of a different species'))
-    }
-
-    if (mergeSpeciesWithHybrids && nrow(hybridParents) > 0) {
-        speciesGroups <- as.list(as.character(unique(allPed$Species)))
-        toMerge <- unique(hybridParents[c('Species.x', 'Species.y')])
-        for (idx in 1:nrow(toMerge)){
-            speciesToCollapse <- as.character(unlist(toMerge[idx,,drop = TRUE]))
-            matchingIdx <- sapply(speciesGroups, function(x){
-                return(length(intersect(speciesToCollapse, x)) > 0)
-            })
-
-            speciesToCollapse <- sort(unique(c(speciesToCollapse, unlist(speciesGroups[matchingIdx]))))
-            speciesGroups <- speciesGroups[!matchingIdx]
-            speciesGroups <- append(speciesGroups, list(speciesToCollapse))
-        }
-
-        print('These species will be merged for processing:')
-        invisible(lapply(speciesGroups, function(x){
-            if (length(x) > 1) {
-                print(x)
-            }
-        }))
-
-        return(speciesGroups)
-    } else {
-        return(unique(allPed$Species[allPed$Species != 'Unknown']))
-    }
-}
-
-validateExpectedKinshipSubset <- function(dataToTest, expectedValues, errorRows, testReciprocal = TRUE) {
-    if (nrow(dataToTest) == 0 || nrow(expectedValues) == 0) {
-        return(errorRows)
-    }
-
-    # Generate the reciprocal of relationships as well:
-    if (testReciprocal) {
-        ret2 <- data.frame(Id = expectedValues$Id2, Id2 = expectedValues$Id, Relationship = expectedValues$Relationship, ExpectedCoefficient = expectedValues$ExpectedCoefficient)
-        ret2$Relationship <- sapply(expectedValues$Relationship, function(x){
-            x <- unlist(strsplit(x, split = '/'))
-            if (length(x) == 1) {
-                return(x)
-            }
-
-            return(paste0(x[2], '/', x[1]))
-        })
-        expectedValues <- dplyr::bind_rows(expectedValues, ret2)
-        rm(ret2)
-    }
-
-    dat <- merge(dataToTest, expectedValues, by = c('Id', 'Id2'), all.x = T, all.y = T) %>%
-      filter(!is.na(ExpectedCoefficient)) %>%
-      filter(is.na(coefficient) | coefficient < ExpectedCoefficient)
-
-    if (nrow(dat) == 0) {
-        return(errorRows)
-    }
-
-    if (all(is.null(errorRows))) {
-        return(dat)
-    }
-
-    return(dplyr::bind_rows(errorRows, dat))
-}
-
-validateExpectedKinship <- function(pedDf, dataToTest) {
-    errorRows <- NULL
-
-    # See reference: https://en.wikipedia.org/wiki/Coefficient_of_relationship#Kinship_coefficient
-    self <- data.frame(Id = pedDf$Id, Id2 = pedDf$Id, Relationship = 'Self', ExpectedCoefficient = 0.5)
-    errorRows <- validateExpectedKinshipSubset(dataToTest, self, errorRows, testReciprocal = FALSE)
-    rm(self)
-
-    parentChild <- dplyr::bind_rows(
-      data.frame(Id = pedDf$Id, Id2 = pedDf$Dam, Relationship = 'Child/Parent'),
-      data.frame(Id = pedDf$Id, Id2 = pedDf$Sire, Relationship = 'Child/Parent')
-    ) %>% filter(!is.na(Id2)) %>% mutate(ExpectedCoefficient = 0.25)
-    errorRows <- validateExpectedKinshipSubset(dataToTest, parentChild, errorRows)
-    rm(parentChild)
-
-    grandParentOffspring1 <- merge(pedDf[!is.na(pedDf$Dam),], pedDf, by.x = c('Dam'), by.y = c('Id'), all.x = F, all.y = F)
-    grandParentOffspring1 <- dplyr::bind_rows(
-      grandParentOffspring1 %>% select(Id, Dam.y) %>% filter(!is.na(Dam.y)) %>% rename(Id2 = Dam.y) %>% mutate(Relationship = 'Grandchild/Maternal Granddam'),
-      grandParentOffspring1 %>% select(Id, Sire.y) %>% filter(!is.na(Sire.y)) %>% rename(Id2 = Sire.y) %>% mutate(Relationship = 'Grandchild/Maternal Grandsire')
-    ) %>% mutate(ExpectedCoefficient = 0.125)
-    errorRows <- validateExpectedKinshipSubset(dataToTest, grandParentOffspring1, errorRows)
-    rm(grandParentOffspring1)
-
-    grandParentOffspring2 <- merge(pedDf[!is.na(pedDf$Sire),], pedDf, by.x = c('Sire'), by.y = c('Id'), all.x = F, all.y = F)
-    grandParentOffspring2 <- dplyr::bind_rows(
-      grandParentOffspring2 %>% select(Id, Dam.y) %>% filter(!is.na(Dam.y)) %>% rename(Id2 = Dam.y) %>% mutate(Relationship = 'Grandchild/Paternal Granddam'),
-      grandParentOffspring2 %>% select(Id, Sire.y) %>% filter(!is.na(Sire.y)) %>% rename(Id2 = Sire.y) %>% mutate(Relationship = 'Grandchild/Paternal Grandsire')
-    ) %>% mutate(ExpectedCoefficient = 0.125)
-    errorRows <- validateExpectedKinshipSubset(dataToTest, grandParentOffspring2, errorRows)
-    rm(grandParentOffspring2)
-
-    fullSibs <- merge(pedDf[!is.na(pedDf$Dam) & !is.na(pedDf$Sire),], pedDf[!is.na(pedDf$Dam) & !is.na(pedDf$Sire),], by = c('Sire', 'Dam'), all.x = F, all.y = F) %>%
-      select(Id.x, Id.y) %>%
-      rename(Id = Id.x, Id2 = Id.y) %>%
-      filter(Id != Id2) %>%
-      mutate(Relationship = 'Full sib', ExpectedCoefficient = 0.25)
-    errorRows <- validateExpectedKinshipSubset(dataToTest, fullSibs, errorRows)
-    rm(fullSibs)
-
-    halfSibs1 <- merge(pedDf[!is.na(pedDf$Dam),], pedDf[!is.na(pedDf$Dam),], by = c('Dam'), all.x = F, all.y = F) %>%
-      filter(Sire.x != Sire.y) %>%
-      select(Id.x, Id.y) %>%
-      rename(Id = Id.x, Id2 = Id.y) %>%
-      filter(Id != Id2) %>%
-      mutate(Relationship = 'Half sib', ExpectedCoefficient = 0.125)
-    errorRows <- validateExpectedKinshipSubset(dataToTest, halfSibs1, errorRows)
-    rm(halfSibs1)
-
-    halfSibs2 <- merge(pedDf[!is.na(pedDf$Sire),], pedDf[!is.na(pedDf$Sire),], by = c('Sire'), all.x = F, all.y = F) %>%
-      filter(Dam.x != Dam.y) %>%
-      select(Id.x, Id.y) %>%
-      rename(Id = Id.x, Id2 = Id.y) %>%
-      filter(Id != Id2) %>%
-      mutate(Relationship = 'Half sib', ExpectedCoefficient = 0.125)
-    errorRows <- validateExpectedKinshipSubset(dataToTest, halfSibs2, errorRows)
-    rm(halfSibs2)
-
-    return(errorRows)
-}
-
-speciesToProcess <- generateSpeciesToProcess(allPed, opts$mergeSpeciesWithHybrids)
-
 newRecords <- NULL
-for (speciesSet in speciesToProcess){
-    allRecordsForSpecies <- allPed[allPed$Species %in% speciesSet,]
-    print(paste0('Processing species set: ', paste0(speciesSet, collapse = ','), ', with ', nrow(allRecordsForSpecies), ' IDs'))
+for (species in unique(allPed$Species)){
+    allRecordsForSpecies <- allPed[allPed$Species %in% species,]
+    print(paste0('Processing species: ', species, ', with ', nrow(allRecordsForSpecies), ' IDs'))
     if (nrow(allRecordsForSpecies) == 1) {
         print('single record, skipping')
         newRecords <- dplyr::bind_rows(newRecords,data.frame(Id = allRecordsForSpecies$Id, Id2 = allRecordsForSpecies$Id, coefficient = 0.5, Species = allRecordsForSpecies$Species))
@@ -219,19 +72,6 @@ for (speciesSet in speciesToProcess){
     temp.tri <- merge(temp.tri, allRecordsForSpecies[c('Id', 'Species')], by = 'Id', all.x = TRUE)
 
     newRecords <- dplyr::bind_rows(newRecords,temp.tri)
-
-    # NOTE: perform per-species to save memory
-    if (opts$performKinshipValidation) {
-        print('Validating coefficients against expected values')
-        errorRows <- validateExpectedKinship(allRecordsForSpecies, temp.tri)
-        if (!all(is.null(errorRows))) {
-            fileName <- paste0('kinshipErrors_', paste0(speciesSet, collapse = '.'), '.txt')
-            print(paste0('There were unexpected kinship values! See the file ', fileName, ' for more information'))
-            write.table(newRecords, file = fileName, row.names = FALSE, quote = FALSE, sep = '\t')
-        } else {
-            print('All coefficients were within expected ranges from predicted values')
-        }
-    }
 }
 
 # write TSV to disk
diff --git a/ehr/resources/web/ehr/panel/GeneticCalculationSettingsPanel.js b/ehr/resources/web/ehr/panel/GeneticCalculationSettingsPanel.js
@@ -42,18 +42,6 @@ Ext4.define('EHR.panel.GeneticCalculationSettingsPanel', {
                         });
                     }
                 }
-            },{
-                xtype: 'checkbox',
-                fieldLabel: 'Merge Species With Hybrids?',
-                itemId: 'mergeSpeciesWithHybrids',
-                listeners: {
-                    render: function (c) {
-                        Ext4.create('Ext.tip.ToolTip', {
-                            target: c.getEl(),
-                            html: 'If any hybrid animals are detected, these species groups will be merged and processed as one unit. Merging all these species together ensures that the correct ancestors from each side are present'
-                        });
-                    }
-                }
             },{
                 xtype: 'numberfield',
                 hideTrigger: true,
@@ -110,7 +98,6 @@ Ext4.define('EHR.panel.GeneticCalculationSettingsPanel', {
         this.down('#hourOfDay').setValue(results.hourOfDay);
         this.down('#containerPath').setValue(results.containerPath);
         this.down('#kinshipValidation').setValue(results.kinshipValidation);
-        this.down('#mergeSpeciesWithHybrids').setValue(results.mergeSpeciesWithHybrids);
         this.down('#allowImportDuringBusinessHours').setValue(results.allowImportDuringBusinessHours)
     },
 
@@ -123,7 +110,6 @@ Ext4.define('EHR.panel.GeneticCalculationSettingsPanel', {
                 enabled: this.down('#enabled').getValue(),
                 hourOfDay: this.down('#hourOfDay').getValue(),
                 kinshipValidation: this.down('#kinshipValidation').getValue(),
-                mergeSpeciesWithHybrids: this.down('#mergeSpeciesWithHybrids').getValue(),
                 allowImportDuringBusinessHours: this.down('#allowImportDuringBusinessHours').getValue()
             },
             method : 'POST',
diff --git a/ehr/src/org/labkey/ehr/EHRController.java b/ehr/src/org/labkey/ehr/EHRController.java
@@ -640,7 +640,7 @@ public ApiResponse execute(ScheduleGeneticCalculationForm form, BindException er
                 errors.reject(ERROR_MSG, "Unable to find container for path: " + form.getContainerPath());
                 return null;
             }
-            GeneticCalculationsJob.setProperties(form.isEnabled(), c, form.getHourOfDay(), form.isKinshipValidation(), form.isMergeSpeciesWithHybrids(), form.isAllowImportDuringBusinessHours());
+            GeneticCalculationsJob.setProperties(form.isEnabled(), c, form.getHourOfDay(), form.isKinshipValidation(), form.isAllowImportDuringBusinessHours());
 
             return new ApiSimpleResponse("success", true);
         }
@@ -760,7 +760,6 @@ public static class ScheduleGeneticCalculationForm
         private int hourOfDay;
 
         private boolean _kinshipValidation;
-        private boolean _mergeSpeciesWithHybrids;
         private boolean _allowImportDuringBusinessHours;
 
         public boolean isEnabled()
@@ -803,16 +802,6 @@ public void setKinshipValidation(boolean kinshipValidation)
             _kinshipValidation = kinshipValidation;
         }
 
-        public boolean isMergeSpeciesWithHybrids()
-        {
-            return _mergeSpeciesWithHybrids;
-        }
-
-        public void setMergeSpeciesWithHybrids(boolean mergeSpeciesWithHybrids)
-        {
-            _mergeSpeciesWithHybrids = mergeSpeciesWithHybrids;
-        }
-
         public boolean isAllowImportDuringBusinessHours()
         {
             return _allowImportDuringBusinessHours;
@@ -840,7 +829,6 @@ public ApiResponse execute(ScheduleGeneticCalculationForm form, BindException er
             ret.put("enabled", GeneticCalculationsJob.isEnabled());
             ret.put("hourOfDay", GeneticCalculationsJob.getHourOfDay());
             ret.put("kinshipValidation", GeneticCalculationsJob.isKinshipValidation());
-            ret.put("mergeSpeciesWithHybrids", GeneticCalculationsJob.isMergeSpeciesWithHybrids());
             ret.put("allowImportDuringBusinessHours", GeneticCalculationsJob.isAllowImportDuringBusinessHours());
 
             return new ApiSimpleResponse(ret);
diff --git a/ehr/src/org/labkey/ehr/pipeline/GeneticCalculationsJob.java b/ehr/src/org/labkey/ehr/pipeline/GeneticCalculationsJob.java
@@ -122,17 +122,6 @@ public static boolean isKinshipValidation()
             return false;
     }
 
-    public static boolean isMergeSpeciesWithHybrids()
-    {
-        Map<String, String> saved = PropertyManager.getProperties(GENETICCALCULATIONS_PROPERTY_DOMAIN);
-
-        if (saved.containsKey("mergeSpeciesWithHybrids"))
-            return Boolean.parseBoolean(saved.get("mergeSpeciesWithHybrids"));
-        else
-            return false;
-    }
-
-
     public static boolean isAllowImportDuringBusinessHours()
     {
         Map<String, String> saved = PropertyManager.getProperties(GENETICCALCULATIONS_PROPERTY_DOMAIN);
@@ -173,14 +162,13 @@ public static Integer getHourOfDay()
         return null;
     }
 
-    public static void setProperties(Boolean isEnabled, Container c, Integer hourOfDay, Boolean isKinshipValidation, Boolean mergeSpeciesWithHybrids, Boolean allowImportDuringBusinessHours)
+    public static void setProperties(Boolean isEnabled, Container c, Integer hourOfDay, Boolean isKinshipValidation, Boolean allowImportDuringBusinessHours)
     {
         PropertyManager.PropertyMap props = PropertyManager.getWritableProperties(GENETICCALCULATIONS_PROPERTY_DOMAIN, true);
         props.put("enabled", isEnabled.toString());
         props.put("container", c.getId());
         props.put("hourOfDay", hourOfDay.toString());
         props.put("kinshipValidation", isKinshipValidation.toString());
-        props.put("mergeSpeciesWithHybrids", mergeSpeciesWithHybrids.toString());
         props.put("allowImportDuringBusinessHours", allowImportDuringBusinessHours.toString());
         props.save();
 
diff --git a/ehr/src/org/labkey/ehr/pipeline/GeneticCalculationsRTask.java b/ehr/src/org/labkey/ehr/pipeline/GeneticCalculationsRTask.java
@@ -104,25 +104,13 @@ public RecordedActionSet run() throws PipelineJobException
     {
         List<RecordedAction> actions = new ArrayList<>();
 
-        actions.add(runScript("populateInbreeding.r", GeneticCalculationsImportTask.INBREEDING_FILE, "Inbreeding Coefficient Output", null));
-
-        List<String> kinshipArgs = new ArrayList<>();
-        if (getJob().getParameters().containsKey("mergeSpeciesWithHybrids") && "true".equalsIgnoreCase(getJob().getParameters().get("mergeSpeciesWithHybrids")))
-        {
-            kinshipArgs.add("-m");
-        }
-
-        if (getJob().getParameters().containsKey("kinshipValidation") && "true".equalsIgnoreCase(getJob().getParameters().get("kinshipValidation")))
-        {
-            kinshipArgs.add("-v");
-        }
-
-        actions.add(runScript("populateKinship.r", GeneticCalculationsImportTask.KINSHIP_FILE, "Kinship Output", kinshipArgs));
+        actions.add(runScript("populateInbreeding.r", GeneticCalculationsImportTask.INBREEDING_FILE, "Inbreeding Coefficient Output"));
+        actions.add(runScript("populateKinship.r", GeneticCalculationsImportTask.KINSHIP_FILE, "Kinship Output"));
 
         return new RecordedActionSet(actions);
     }
 
-    public RecordedAction runScript(String scriptName, String outputFileName, String actionLabel, @Nullable List<String> extraArgs) throws PipelineJobException
+    public RecordedAction runScript(String scriptName, String outputFileName, String actionLabel) throws PipelineJobException
     {
         PipelineJob job = getJob();
         FileAnalysisJobSupport support = (FileAnalysisJobSupport) job;
@@ -146,10 +134,6 @@ public RecordedAction runScript(String scriptName, String outputFileName, String
         args.add(scriptPath);
         args.add("-f");
         args.add(tsvFile.getPath());
-        if (extraArgs != null)
-        {
-            args.addAll(extraArgs);
-        }
 
         getJob().getLogger().info("Using working directory of: " + support.getAnalysisDirectory().getPath());
         ProcessBuilder pb = new ProcessBuilder(args);
diff --git a/ehr/src/org/labkey/ehr/pipeline/GeneticCalculationsRunnable.java b/ehr/src/org/labkey/ehr/pipeline/GeneticCalculationsRunnable.java
@@ -86,8 +86,6 @@ private void startCalculation(User u, Container c, boolean allowRunningDuringDay
             String xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
                 "<bioml>\n" +
                     "\t<note label=\"allowRunningDuringDay\" type=\"input\">" + allowRunningDuringDay + "</note>" +
-                    "\t<note label=\"kinshipValidation\" type=\"input\">" + GeneticCalculationsJob.isKinshipValidation() + "</note>" +
-                    "\t<note label=\"mergeSpeciesWithHybrids\" type=\"input\">" + GeneticCalculationsJob.isMergeSpeciesWithHybrids() + "</note>" +
                 "</bioml>";
 
             AbstractFileAnalysisProtocol<?> protocol = factory.createProtocolInstance(protocolName, "", xml);
