Generalizing Room.properties.energy.scale() method?

[ed-p-may]

Hi all,

I wonder if you would be ok with revising the RoomEnergyProperties.scale() method in order to make it generalized to all it's attributes with a .scale method?

Scenario

As part of the Honeybee-PH plugin, we have few new objects which are part of the room.properties.energy.hvac and room.properties.energy.shw attributes which actually carry along geometric information. Notably, some ventilation system ducting, and hot-water-piping objects. These objects have geometry (Ladybug LineSegment3D) and I would like to properly scale them during Honeybee model.scale() operations.

Right now, the .scale() method successfully passes down through the model, rooms, properties, but due to the current RoomEnergyProperties.scale() method, it 'stops' at the Room.properties.energy level, since right now only daylighting_control gets scaled.

Proposed Revision

Unless you think its a bad idea for some reason, I would proposed revising the RoomEnergyProperties.scale() method such that it will call .scale on any of its child attributes which include the "scale" method name. So perhaps something very similar to the existing .scale() implementation in honeybee's _Properties, roughly like the following:

# honeybee_energy/properties/room.py

class RoomEnergyProperties(object):
    ...

    def scale(self, factor, origin=None):
        """Scale this object by a factor from an origin point.

        Args:
            factor: A number representing how much the object should be scaled.
            origin: A ladybug_geometry Point3D representing the origin from which
                to scale. If None, it will be scaled from the World origin (0, 0, 0).
        """
        # -- Remove the explicit call to daylighting_control.scale()
        # if self.daylighting_control is not None:
        #     self.daylighting_control.scale(factor, origin)

        for atr_name in dir(self):
            # Do not call .scale() on any of the protected attributes directly, only on the properties
            if atr_name.startswith("_"):
                continue

            # Do not call .scale() on the Host room
            if atr_name == "host":
                continue

            # Get the attribute
            var = getattr(self, atr)

            # Do not call .scale() on attributes that do not have a .scale() method
            if not hasattr(var, "scale"):
                continue
            
            try:
                var.scale(factor, origin)
            except Exception as e:
                import traceback
                
                traceback.print_exc()
                raise Exception("Failed to scale {}: {}".format(var, e))

I believe this would allow for any child objects to carry on the scale operation.

If you think this change would be ok, I can add PR with the proposed changes.

thanks!
-Ed @PH-Tools

[chriswmackey]

Hi @PH-Tools ,

Sorry for the very late response here. You have the right idea here but your current proposal would result in the HVAC being scaled multiple times for each Room that it is applied to (given that it's often the same HVAC Python instance that is assigned to multiple Rooms).

The correct place to assign it is on the ModelEnergyProperties and not the RoomEnergyProperties. And you would have to call ModelEnergyProperties.hvacs to get a list of all unique HVACs in the Model, which can then be scaled.

I think there's also probably a better way to get the extension attributes using an extension_attributes property similar to what we do here in honeybee-core:

https://github.com/ladybug-tools/honeybee-core/blob/master/honeybee/properties.py#L31-L34

Lastly, if you implement this scale() capability, you should probably also implement it for the 4 other types of transforms that honeybee objects support, including:

• move
• rotate
• rotate_xy
• reflect

If you put together a PR that addresses all of this, I am happy to review it and merge.

[ed-p-may]

Hi @chriswmackey ,

Sorry for the crazy long delay on this! It fell off my critical TODO list, but has popped back up. If you are still open to it, I have opened a PR with the proposed changes.

I think I implemented everything the way you suggested, but of course let me know what you think?

thanks
@ed-p-may