acope3 / RibModelFramework

1 stars 14 forks source link

Can't run code with obs phi values #395

Closed mikegilchrist closed 10 months ago

mikegilchrist commented 10 months ago

Both the CRAN and more recent versions of the code seg fault when gene expression data is included with the fit.

acope3 commented 10 months ago

I think both versions are broken, but for different reasons.

CRAN version: I ran the tests from the version that should be on CRAN and it passed. We have a test for both with and without $\Phi$ (observed gene expression data) runs but it seg faulted when I applied it to my real data. The difference between the test and real datasets is the latter had 3 measurements while the former only had 1. The code is intended to allow for multiple measures of gene expression, but it is only formally being tested for 1. We can update the test with little issue. @mikegilchrist does your dataset have more than 1 measurement of gene expression and if so, what happens if you reduce it to a single dataset?

More recent version: As we expanded the models used in AnaCoDa, we needed to expand the functionality of the code to allow for observed estimates of gene expression for other models. Originally, this code was ROC-specific. Ideally, these functions would be part of the Model class (the parent class) as they are general to all models. Moving this code to the Model class appears to be the issue. If I keep the code in the ROCModel class, things seem to work fine based on some implementation tests. I think this has to do with the parent Model object being unable to "see" the derived class Parameter object. I Will need to do some more digging to figure out exactly why this is. In the meantime, a simple functional solution is to just have repetitive functions for each of the Model classes (ROC, FONSE, etc.), which was how the code originally was. This kinda flies in the face of the idea of object-oriented programming and polymorphism, but might be an okay temporary solution for our next submission to CRAN.

acope3 commented 10 months ago

@mikegilchrist Please install the version of AnaCoDa from branch with_phi_segfault-395 and let me know if this resolved the issue on your end. I was able to run it on my end without any issues.

mikegilchrist commented 10 months ago

I will try the suggested version out, but I will note that the CRAN version crashed on me as well and I only had 1 observation/gene. Perhaps this is telling us there's a different issue, possibly on my end with my data.

mikegilchrist commented 10 months ago

Figured out why I saw the behavior with the CRAN version (long story...). with_phi_segfault-395 seems to work now as well. I do, however, get errors when I run testthat for the [with_phi_segfault-395] branch.

acope3 commented 10 months ago

Figured out why I saw the behavior with the CRAN version (long story...). with_phi_segfault-395 seems to work now as well. I do, however, get errors when I run testthat for the [with_phi_segfault-395] branch.

I noticed this, as well. This is an unrelated issue that has to do with a change I made to how the base Parameter class specifies the number of codon-specific parameters. This used to be either 40 or 41 depending on the value of split.serine, but it makes more sense to me that this value should be initialized in the derived Parameter classes (ROC, PANSE, etc.) because this number is dependent on the model. Current tests do not reflect this change. I will create a separate issue ticket for this. Closing this issue