DiODeProject / MuMoT

Multiscale Modelling Tool - mathematical modelling without the maths
https://mumot.readthedocs.io/
GNU General Public License v3.0
22 stars 6 forks source link

SSA/multiagent force initial state widgets to sum to 1 even if system size is not constant #318

Closed jarmarshall closed 5 years ago

jarmarshall commented 5 years ago

See also issue #317 - however in this case the user cannot correct the error, since the slider update logic inappropriately enforces this constraint.

Is it a feature of SSA() that non-constant system size models cannot be simulated? In which case SSA needs to check, and raise an exception if the model is not suitable for the method.

multiagent() with moving particles (which the method apparently defaults to for this class of model) exhibits the same problem.

jarmarshall commented 5 years ago

N.B. since release 1.0.0 has already been deployed this will need to be a patch release (1.0.1)

joefresna commented 5 years ago

I can unlock the sum to 1. However, it looks weird to me that a user specifies a system with size S=20 and then initialises it with (for instance) 3 times the size S, i.e. with 60 agents.

joefresna commented 5 years ago

At the moment, SSA and multiagent allow the system to grow over time above the system size, however, the initial state was locked to sum 100% of S.

jarmarshall commented 5 years ago

can unlock the sum to 1. However, it looks weird to me that a user specifies a system with size S=20 and then initialises it with (for instance) 3 times the size S, i.e. with 60 agents.

I agree that sounds strange - can that not be changed?

joefresna commented 5 years ago

at the moment, the initial state sum is fixed to 1. But this issue is asking to unlock to > 1...

jarmarshall commented 5 years ago

So you're saying that actually it makes some sense to force the sum to 1 since it refers only to the initial conditions?

Is there a branch I can checkout to see how it's working now?

joefresna commented 5 years ago

it is already working like this without change

jarmarshall commented 5 years ago

So we think this and issue #317 are actually non-issues, now issue #352 has been separated out?

joefresna commented 5 years ago

I'm working to make possible to have initial state >1 for other views (e.g. integrate())

joefresna commented 5 years ago

We need to decide if we want this possibility active also for SSA() and multiagent()

jarmarshall commented 5 years ago

Well now I'm not sure we should do so for either - integrate() also works with system size in conjunction with the proportions sliders. I think actually we can close this and #317 now?

joefresna commented 5 years ago

Too late, I half implemented it... ...I think it can be useful for some systems, and it can also be useful for computing noise around fixed points because it calls the SSA on the background...

However, I may keep it locked to 1 when the user simply calls SSA() or multiagent()

joefresna commented 5 years ago

I changed my old print("Warning message") with raise exceptions.MuMoTWarning("Warning message") (as indicated in #352), however this stops the program. Instead, the desired behaviour is to let it keep running and just print the warning message. How shall I do that? Do you know, @jarmarshall ?

jarmarshall commented 5 years ago

Moving the discussion on warnings to #352

Not clear to me that the new functionality makes sense - SSA noise magnitude is determined by system size, no? Just because something has been implemented doesn't mean it should be merged...

joefresna commented 5 years ago

Ok, I just committed https://github.com/DiODeProject/MuMoT/commit/7c6a86990c998549e9721eb6fdb1d1a03058dfb6 a change that allows sum > 1 for integrate(), bifurcation() and noiseCorrelations(). (I remember @tbose1 has good reasons for wanting this)

At the moment SSA() and multiagent() are instead forced to have the sum of the initial system size equal to 1

jarmarshall commented 5 years ago

OK, since you wrote the code but I don't yet understand why it is useful could you please set fixSumTo1 to be true for all views for now, and if we figure out when it is useful, or @tbose1 reminds us, we can change that.

Please create a pull request for this to close this and issue #317