KarrLab / wc_lang

Language for describing whole-cell models as reaction networks
MIT License
9 stars 3 forks source link

Switching off requirement for uniqueness for SpeciesCoefficient #137

Closed YinHoon closed 4 years ago

YinHoon commented 4 years ago

Th requirement for uniqueness of the (species, coefficient) pair is causing validation error during model preparation for simulation. The error is encountered even when the SpeciesCoefficient instance is reused among multiple reactions.

File "/root/host/Documents/wc_sim/wc_sim/simulation.py", line 122, in run self._prepare() File "/root/host/Documents/wc_sim/wc_sim/simulation.py", line 89, in _prepare raise MultialgorithmError(indent_forest(['The model is invalid:', [errors]])) wc_sim.multialgorithm_errors.MultialgorithmError: The model is invalid:

SpeciesCoefficient: 'coefficient': Combinations of (coefficient, species) must be unique across all instances of this class. The following combinations are repeated:
1.0, Complex_91[n]

YinHoon commented 4 years ago

The unique_together line in SpeciesCoefficient has been commented out.

jonrkarr commented 4 years ago

Please revert this change. This is necessary to enable comparisons between models. The model construction needs to be changed to point to the same instances of SpeciesCoefficients.

YinHoon commented 4 years ago

The issue was encountered even when model construction reuse the same instances as follows:

objects = {
            wc_lang.core.SpeciesType: {i.id: i for i in model.species_types},
            wc_lang.core.Compartment: {i.id: i for i in model.compartments},
            wc_lang.core.Species: {i.id: i for i in model.species},
            }
all_species_coef = {'{}{}'.format(j.coefficient, j.species.id):j for i in model.reactions for j in i.participants}
attr = wc_lang.core.ReactionParticipantAttribute()

for Id, submodel, framework, rxn, rate in added_rxns:
        model_submodel = model.submodels.get_or_create(id=submodel,
            framework=wc_ontology[framework])  
        model_rxn = model.reactions.create(submodel=model_submodel, id=Id, reversible=False)            
        parts, error = attr.deserialize(rxn, objects)
        assert error is None, str(error)
        for part in parts:
            part_str = '{}{}'.format(part.coefficient, part.species.id)
            if part_str in all_species_coef:
                model_rxn.participants.add(all_species_coef[part_str])
            else:    
                model_rxn.participants.add(part)
jonrkarr commented 4 years ago

This code is hard to understand. Please clarify.

YinHoon commented 4 years ago

The code above adds new reactions read from an excel file to an existing model. The new reaction equation strings (rxn) are deserialized using 'parts, error = attr.deserialize(rxn, objects)' where 'objects' is the dictionary of object instances from the existing model. The species and coefficients created from deserialization are checked against existing SpeciesCoefficient instances (all_species_coeff). If the coefficient exists, it will be reused in 'model_rxn.participants.add(all_species_coef[part_str])'

I can alternatively implement the above using:

model_rxn.participants.add(part.species.species_coefficients.get_or_create(coefficient=part.coefficient))

which will be cleaner. Is there some internal code within obj_tables that only allow reuse through 'get_or_create' but will cause errors if the objects are pointed to using other methods?

jonrkarr commented 4 years ago

Unformatted code is hard to read. Please use markdown syntax (e.g., ```...```) to format code. Content inside backticks should not contain additional markup (e.g., ** for bolding). See my revision of your post for an example.

I don't understand the question above.

There's a few issues with the initial code:

I think this may address the issue. I have also renamed the variables for clarity.

import obj_tables.utils
import wc_lang

cls_objs = obj_tables.utils.group_objects_by_model(model.get_related())
cls_to_id_to_obj = {cls: {obj.serialize(): obj for obj in objs} for cls, objs in cls_objs.items()}

for rxn_id, submodel_id, submodel_framework, rxn_equation, rxn_rate in added_rxns:
    submodel = model.submodels.get_or_create(id=submodel_id,
        framework=wc_ontology[submodel_framework])
    rxn = model.reactions.create(submodel=submodel, id=rxn_id, reversible=False)            
    rxn.participants, error = wc_lang.Reaction.participants.deserialize(rxn_equation, cls_to_id_to_obj)
    assert error is None, str(error)
YinHoon commented 4 years ago

Thanks for the suggested code. The issue is now resolved and the question above is no longer valid. I have reverted the change.