Added metaclass to create dummy Paramter/Variables#298
Added metaclass to create dummy Paramter/Variables#298Jhsmit wants to merge 1 commit intotBuLi:masterfrom Jhsmit:dummy
Conversation
|
I like the idea for creating this, but I would rather choose a solution that stays closer to sympy to prevent problems in the future. I think the following object structure should work: BaseParameter - defines slots min, max, fixed, value. This is a mixin class and doesn't inherit from anything. I think this should work, what about you? |
|
Yes I agree this PR should not be merged under any circumstances. But for it a was a fun trip into the madness that is symfit, sympy and metaclasses. The structure you proposed is indeed the best solution in my opinion. I dont remember the details but I had trouble envisioning it because of I ran into some MRO issues. But as a mixin it might work. We'd also need to do the same thing for Variables then but that should be much easier. |
|
Yeah I had those same problems when trying to include indexed variables and parameters and back then I eventually found that a mixin was the only way of getting it to work if you didn't want to go down a very dark path. I also don't think the solution above should be very hard to implement, and I would prefer having DummyParameters and DummyVariables as seperate classes so we don't create unexpected problems. And we can add a |
|
Perhaps Parameters, Variables should always be Dummy's in a future (major) version of symfit? Perhaps you could consider someone wanting to do this: model = {
Variable('y1'): Parameter('a')*Variable('x') + Parameter('b'),
Variable('y2'): Parameter('a')*Variable('x')**2
}Which with dummy variables you would have to define by: y1_var = Variable('y1')
y2_var = Variable('y2')
a_par = Parameter('a')
b_par = Parameter('b')
model = { ...Where I do agree that the former has a nicer syntax and the latter pollutes the local scope but this example ignores the fact that on parameters you have to define initial guesses so usually you cannot write your problem as option 1 anyway. EDIT: forgot to mention here that if I naively make IndexedParameter/Variables for my own internal use from |
This is a bit of a hack, but it allows users to force creation of dummy Parameter / Variables instead of symbol ones (see #290).
Usage:
Where in normal
symfitbehaviour the result would be(3, 3).Why would you want to use this? Well, if your application of
symfitis one script, one model kind of fitting then there is probably very little use. But if you make a bigger application where many models are created and active at the same time (thread safe?) then dummy behaviour is desired.Also, lets say you happen to be making your parmeters like this:
Without dummies, these two parameters are the same, and fitting fails without an error. With dummies, it still doesnt work, but at least you get an error :)
Yes, I agree the implementation is a bit sketchy and could be considered dark magic. Suggestions are welcome. Maybe some split of the inheritance into
ParameterandDummyParameter, but because of all the implementations of__new__it seems thatDummysomehow needs to be introduced just afterArgumentin the mro.The hack via
builtinsis needed because the information on wheter to dummy or not needs to be passed before any symfit imports as that is when the classes are created.builtinsis py3 only (did we drop py2 already?)