jrleeman / rsfmodel

Rate and state frictional solver
MIT License
31 stars 12 forks source link

Improve decoupling #2

Closed dopplershift closed 9 years ago

dopplershift commented 9 years ago

Looks good so far. I see one part of the design that I would change. Right now, all of the different models are implemented in the main class:

    def dieterichState(self):
        return 1. - self.v * self.theta / self.dc

    def ruinaState(self):
        return -1*(self.v * self.theta / self.dc)*log(self.v * self.theta / self.dc)

    def przState(self):
        return 1. - (self.v * self.theta / (2*self.dc))**2

I would change this in one of two ways. First way, use inheritance to implement new models:

class RateState(object):
    # snip other code
    def dtheta_dt(self):
       raise NotImplementedError('dtheta_dt Needs to be implemented by subclass')

class Dieterich(RateState):
    def dtheta_dt(self):
        return 1. - self.v * self.theta / self.dc

# And other classes

In general, this would let you later add more methods to encapsulate other parts of the models (maybe calculation of dmu_dt). On the other hand, for just encapsulating models of dtheta_dt, you could just use functions, since it looks like all of them only need a few variables:

def dieterichState(v, theta, dc):
    return 1. - v * theta / dc

class RateState(object):
    # snip other code
    def _integrationStep(self, w, t):
        # SNIP
        dtheta_dt = self.stateLaw(self.v, self.theta, self.dc)

Besides being lightweight, this could also let you mix and match different functions for dtheta_dt and dmu_dt.

jrleeman commented 9 years ago

I like the inheritance approach, but think the simple function tack may be more versatile in the end. It's hard to predict how people will want to use/abuse this. I'll do this now.

dopplershift commented 9 years ago

Inheritance would be a clear for many languages; one of the things about Python is that, given the way it handles functions, you can make just as clean a design using functions.