sys-bio / temp-biomodels

Temporary place for coordination of updating existing Biomodels
Creative Commons Zero v1.0 Universal
2 stars 2 forks source link

Updated Copasi output: 637 #109

Closed luciansmith closed 2 years ago

luciansmith commented 2 years ago

Copasi now generates different output for biomodels 638, which makes one of our replacements out of sync. Our current replacement rule is:

BIOMD0000000637:
  - filename: 'Bush2016-Simplified-Carrousel-model-of-GPCR.sedml'
    type: replace_text
    old: >
      <dataSet id="ds_4_task3" dataReference="_1_task3"/>
    new: >
      <dataSet id="ds_4_task3" label="Gbg" dataReference="Gbg_1_task3"/>

But now the dataSets are:

      <listOfDataSets>
        <dataSet id="ds_1_task3" label="k_off_R_G" dataReference="k_off_R_G_1_task3"/>
        <dataSet id="ds_2_task3" label="Rtot" dataReference="Rtot_1_task3"/>
        <dataSet id="ds_3_task3" label="L" dataReference="L_1_task3"/>
        <dataSet id="ds_4_task3" label="tot_LR" dataReference="tot_LR_1_task3"/>
      </listOfDataSets>

The original final version of the SED-ML was:

      <listOfDataSets>
        <dataSet id="ds_1_task3" label="k_off_R_G" dataReference="k_off_R_G_1_task3"/>
        <dataSet id="ds_2_task3" label="Rtot" dataReference="Rtot_1_task3"/>
        <dataSet id="ds_3_task3" label="L" dataReference="L_1_task3"/>
        <dataSet id="ds_4_task3" label="Gbg" dataReference="Gbg_1_task3"/>
        <dataSet id="ds_5_task3" label="tot_LR" dataReference="tot_LR_1_task3"/>
      </listOfDataSets>

So, as far as I can tell, the original fix involved taking a broken dataSet, and replacing it with a new one that Copasi didn't originally put in that report. In the latest version, Copasi doesn't put the broken dataSet in the report in the first place, and we thus can't replace it with a new one.

@jonrkarr : do we want to insert Gbg into the report anyway? It's currently in the figure but not the report. (Also note that we now have a new report with all the data from task3, if that's the intent of the original change.)

New versions of SED-ML and SBML (without the new dataSet) attached.

BIOMD0000000637_url.xml.txt Bush2016-Simplified-Carrousel-model-of-GPCR.sedml.txt

luciansmith commented 2 years ago

(Basically the same thing happens for 638. New Copasi doesn't make incorrect data generators, and doesn't include them in the report.)

jonrkarr commented 2 years ago

Sounds like COPASI introduced a new bug while trying to fix another. That's unfortunate. Sounds like we should report this bug.

I have no strong feelings either way about correcting for the new COPASI bug or not. Ideally, we would try to deliver as best as SED-ML as we can -- accurate reflection of the intended simulation and valid. At the same time, we only have finite bandwidth. I think we're tackling the valid part. To manage our time, I think we want to be able to trust tools such as COPASI to create accurate SED-ML.

I'd be fine with leaving this to COPASI to fix. I'm ok with accepting that it might take BioModels/COPASI a while to fix this. Perhaps we could append a note about this to the curation note.

luciansmith commented 2 years ago

No, I don't think it's a new bug. It looks to me like there was an incorrect "_1_task3" object that was in the data generators and report, which Copasi removed. Separately, we had added in "Gbg" to the same report with the error. Do we want it there still? I don't know why it was added originally; it might have been semantically important, or it may have just felt like a good idea to us-as-curators.

jonrkarr commented 2 years ago

(I haven't examined the original versions of these files from BioModels.)

I think we can safely assume the curator intended the COPASI file to be the authoritative reference. I think its unlikely that the curator intended to improve the simulation by editing the SED-ML after it was exported by COPASI. If the SED-ML wasn't aligned with the COPASI, I think we can safely correct the SED-ML to align with the COPASI file.

Summary,

luciansmith commented 2 years ago

Works for me! Thanks for the input.

luciansmith commented 2 years ago

Fixed (along with other things) in https://github.com/sys-bio/temp-biomodels/commit/a7183db1be3607683059cc2cfea50bade422e29c

There was indeed a minor bug in Copasi's otherwise improved code, so we still need replacements for this file, but it's not as extensive.