crestdsl / CREST

Continuous REactive SysTems DSL
https://crestdsl.github.io
MIT License
18 stars 2 forks source link

Problem when overriding already connected subentity in __init__ #15

Closed gmarthe closed 5 years ago

gmarthe commented 5 years ago

There is still no output for the LivingRoom cell (at the end of "ErrLiving") but this time there is no inherance. Maybe it comes from Transitions again but I didn't find why. It would be perfect if there can be an error explanation for this kind of problems.

The ErrLiving file and the ones to run it can be found in the LivingRoomError.zip

LivingRoomError.zip

stklik commented 5 years ago

This is the code of the entity:

class LivingRoom(Entity):
    """ - - - - - - - PORTS - - - - - - - - - - """
    light_In = Input(Resources.light, 200)
    windows = Local(Resources.integer, 3)

    """ - - - - - - - SUBENTITIES - - - - - - - - - - """
    light_controls = LightController()
    electricity_splitter = Splitter()    
    water_splitter = WaterSplitter()
    television = TV()
    vacuum_robot = Vacuum_Robot()
    dishwasher = Dishwasher()

    def __init__(self, number_windows=0, number_lamps=0):
        self.windows.value = number_windows

        installed_devices = []
        for i in range(number_lamps):
            lamp = api.add(self, f"lamp_{i+1}", Lights())
            installed_devices.append(lamp)
            api.add(self, f"connect_lamp_{i+1}_switch", Influence(source=self.light_controls.out, target=lamp.switch_in))

        installed_devices.append(self.television)
        installed_devices.append(self.vacuum_robot)
        installed_devices.append(self.dishwasher)
        # CONNECTION OF DISHWASHER HERE
        self.electricity_splitter.connect_devices(installed_devices)

        api.pullup(presence=self.light_controls.presence_in, 
               lightswitch=self.light_controls.switch_in,
               light_outside=self.light_controls.luminosity_in)

        api.pullup(self.electricity_splitter.electricity_in,
                  self.electricity_splitter.req_electricity_out)

        api.dependencies((self.req_electricity_out, self.lightswitch),
                           (self.req_electricity_out, self.presence))

        api.add(self, "influence_vacuum", Influence(source = self.presence, target = self.vacuum_robot.presence_In))

        water_devices = []

        # PROBLEMATIC REPLACEMENT OF DISHWASHER HERE
        dishwasher = api.add(self, "dishwasher", Dishwasher())
        water_devices.append(dishwasher)

        self.water_splitter.connect_water_devices(water_devices)        
        api.pullup(switch_DW = self.dishwasher.switch_usageDW)        
        api.pullup(self.water_splitter.water_in, self.water_splitter.req_water_out)        
        api.dependencies((self.req_water_out, self.switch_DW))

    """ - - - - - - - STATES & TRANSITIONS - - - - - - - - - - """                
    state = current = State()

elk.plot(LivingRoom(4,6))   

The issue has to do with the fact that the entity's dishwasher subentity is replaced after it has already been connected to the electricity splitter. Thus, replacing removes the port of the electricity_splitter from where an influence is started.

There should probably be a warning message when overriding things in an __init__ (Option 1). Alternatively, we could try to automatically fix it (Option 2).

Option 1: could be feasible since writes to subentity ports are easily detectable (just check targets of influences, updates and actions). Reads from subentity outputs (such as here) are only statically mentioned in influences (source). Update and Action reads are not interesting, since they only access ports at runtime (i.e. by name). Thus, they always reference the correct port anyway. Solution: implement detection and throw an error !

Option 2: would have to identify all connections to the subentity, take the connections and "move" the reference to the original port (by name) and adding it to the Solution: raise a warning and implement the switch automatically. Problem: might enable bad habits, hard to debug issues, etc. It also allows moving too much bad black magic into the __init__ function so that the code becomes weird.

stklik commented 5 years ago

After checking with colleagues, Option 1 is better. (Let's teach users not to rely on black magic)