search

Open-Systems-Pharmacology / MoBi

MoBi® is a software tool for multiscale physiological modeling and simulation
Other
31 stars 10 forks source link

AdjustFormulasVisitor implementation issues #399

Open msevestre opened 5 years ago

msevestre commented 5 years ago

1- Two formula with different names may be mapped as equal if they have the same formula and the same references. I think this is an error

2- Formula check does not check for dimension. I also think this is somewhat an error

@ThomasGaub: Do you remember if not checking the name is a feature or a bug?

msevestre commented 5 years ago

That's the check that is suspicious here https://github.com/Open-Systems-Pharmacology/MoBi/blob/4e314e1b2174859a911dd0327e57516cd1552477/src/MoBi.Core/Services/AdjustFormulaVisitor.cs#L179

UlrichSi commented 5 years ago

I guess that this might be part of a process to unify identical formulas in case they were imported. So, if I imported the same parameter for different molecules one after the other. Or if I duplicated a molecule by importing it including all parameters, renamed the molecule, and confirmed the suggested renaming of parameter formulas. I remember that @ThomasGaub explained this to me many moons ago.

msevestre commented 5 years ago

I understand the idea. However when two formulas have different names, they should not be unified don't you think? This leads to very weird result with a parameter pointing to the formula of another parameter suddenly after being imported.

msevestre commented 5 years ago

Plus the dimension check missing actually prevents the formula for being displayed

UlrichSi commented 5 years ago

I agree, the procedure in place may have side effects, in particular when editing a formula that is used multiple times at unexpected locations. Don't know what is better. Maybe implement the import like the parameter/reaction/molecule names: Ask the user. That might, however, lead to so many queries that a user loses track on them.

msevestre commented 5 years ago

I do not see how replacing a formula with a different name with another formula can be the desired behaviour. I understand checking for similarity to ensure that we do not duplicate the same formula multiple time. But if I have a formula called PARAM1_Formula and another one called PARAM2_Formula with the same formula and ref, and I import those, suddenly one is replaced by the other. Simply weird

StephanSchaller commented 5 years ago

But if I have a formula called PARAM1_Formula and another one called PARAM2_Formula with the same formula and ref, and I import those, suddenly one is replaced by the other. Simply weird

I agree. I don't see why this would be the intended behaviour. Normally same formulas are also called the same...