xarray-contrib / xarray-simlab

Xarray extension and framework for computer model simulations
http://xarray-simlab.readthedocs.io
BSD 3-Clause "New" or "Revised" License
73 stars 9 forks source link

add test for cycle #171

Closed feefladder closed 3 years ago

feefladder commented 3 years ago

While fumbling around, I stumbled upon a cycle. Now I know the codebase says "maybe remove this check because it is not possible", but it is:

@xs.process
class CycleStepOne:
    a = xs.variable(intent="out",global_name = "a")
    group = xs.group("b")

    def initialize(self):
        a = 5

    def run_step(self):
        self.a = np.sum(b for b in self.group)

@xs.process    
class CycleStepTwo:
    a = xs.global_ref("a")
    b = xs.variable(intent="out",groups="b")

    def initialize(self):
        self.b = self.a

    def run_step(self):
        self.a = self.b

cyclic_model = xs.Model({
    "one":CycleStepOne,
    "two":CycleStepTwo
})

raises

RuntimeError: Cycle detected in process graph: one->two->one

I also have a model, that sort of needs this structure.:

@xs.process
class dBiomass:
    """calculates biomass change based on light intensity
    """
    eff_light = xs.variable(description='light use efficiency', default=3)
    biomass = xs.variable(intent='out',description='grown biomass',default=0)
    leaf_ara=xs.variable(intent="out")

    def initialize(self):
        self.leaf_area=0.01
        self.biomass=0.01

    @xs.runtime(args='step_delta')
    def run_step(self,dt):
        ddays = dt / np.timedelta64(1,'D')
        self.intercepted_light = 1-np.exp(-0.8*self.leaf_area)
        self._d_biomass = self.eff_light*ddays*self.intercepted_light#*self.maxrad
        self._d_leaf_area = .5*self._d_biomass

    def finalize_step(self):
        self.biomass += self._d_biomass
        self.leaf_area += self._d_leaf_area

Is there a way to refactor this (to allow for different ways of calculating leaf area, while not rewriting the entire model)? Or is this now the smallest possible process, because of the biomass->leaf_aera->intercepted_light->biomass dependency?

feefladder commented 3 years ago

oops tehre is already a test