Ticket #4214 (closed major enhancement: fixed)
Change variable ordering in rules
|Reported by:||jdsiiro||Owned by:||wehart|
OK, so I realize that this appears to be a very disruptive change... but it has been bothering me for a while now, and I now have hard data as to why this is really not a good practice -- plus a solution that maintains full backwards compatibility.
The problem: Why are all rules defined as indices first, then the model?
- This is counter-intuitive from a programming standpoint: in all languages (including Python) you always put the variable-length arguments at the end of the argument list. That is, we should do "model, I, J, ...".
- This is SLOWER. We spend a measurable amount of time (one-third of constraint.construct()'s direct time) taking the indices, converting them to a list, adding the model to the end, and converting back to a tuple to pass to the rule.
- This places a restriction on rules: either all invocations of a rule must use the same number of indices, or the rule must explicitly take *args and remove model from the end itself. If the model came first, they could define the rule as: "def _rule(model, *index):"
Here's the best part: we can change the semantics and still maintain backwards compatibility using a try-catch block (since transposition of model and index is virtually guaranteed to raise an exception). Old-style models would still run (albeit, slightly slower), but new-style models would be both cleaner and faster.
Timings (constraint.construct() for pmedian.test7.dat):
original: 0.370 sec new with old-style rule: 0.566 sec new with new-style rule: 0.246 sec
Proposed change (implemented within constraint.construct):
if index.__class__ is tuple: try: expr = _self_rule(self.model, *index) except: tmp = list(index) tmp.append(self.model) try: expr = _self_rule(*tmp) except: # Re-run new style and allow exception to propagate expr = _self_rule(self.model, *index) else: try: expr = _self_rule(self.model, index) except: try: expr = _self_rule(index, self.model) except: # Re-run new style and allow exception to propagate expr = _self_rule(self.model, index) if expr is not None: self.add(index, expr)
- Owner changed from wehart to jdsiiro
- Status changed from new to assigned
- Status changed from assigned to closed
- Resolution set to fixed