del_component issue when deleting constraints

[tony121psu]

I'm working on a project that frequently requires solving a model, modifying a constraint, and then resolving. I've been using the del_component & add_component commands for removing and recreating constraints (see simple attached example). It appears that the del_component(constraint of interest) command is leaving behind some garbage if the constraint to be removed is indexed. This necessitates an additional command (i.e., "model.del_component(model.constraint1_index)") to avoid an error.

Is this a bug? Can it be fixed? Should I be doing something differently?

Thanks! Tony Burgard (NETL)

Simple Example.py.txt

[jsiirola]

Yes. I think that this behavior is a bug and can be corrected. I'll take a look and see how easy it is to fix. The only thing that I worry about is that it is possible that another component could rely on the implicit indexing set - but that would require some strange coding on the part of the modeler, so I think it should be safe to automatically remove the implicit sets that are created by indexed components.

There is also a workaround: if you create the indexed component using a single set that is already on the model, then no implicit sets will be created.

[tony121psu]

Thanks much for the fast response. It’d be definitely be very helpful if Pyomo could automatically remove the implicit sets created by the indexed components. Thanks also for the workaround. It seems to be working though it’ll be pretty tedious (especially compared to GAMS) to create a new set each time I want to use a modified constraint.

All the best, Tony

[jsiirola]

To clarify: you don't need to create a new set for every modified instance of a Constraint: you can re-use the Set. The problem that you are hitting in the example that you posted is that all Indexed Pyomo Components are indexed by a single Pyomo Set. When you pass a non-Set (with a capital 'S') to a Pyomo component, Pyomo will implicitly create and populate a Pyomo Set for you; that is:

model.y = ['V1','V2','V3']
model.constraint1 = Constraint(model.y,rule=constraint1_rule)    

creates an implicit Set constraint1_index that is equivalent to Set(initialize=model.y). If you had instead done this:

model.y = Set(initialize=['V1','V2','V3'])
model.constraint1 = Constraint(model.y,rule=constraint1_rule)    

then no implicit set would have been created, and the following would work as expected:

model.del_component(model.constraint1);
model.add_component("constraint1",Constraint(model.y,rule=constraint1_rule))

as would:

model.del_component(model.constraint1);
model.constraint1 = Constraint(model.y,rule=constraint1_rule)

As a final note, the same thing happens when you declare a component with an implicit cross product of sets; that is:

model.con = Constraint(model.A, model.B, rule=con_rule)

will still create an implicit Set to hold the cross product. To avoid the implicit set, create a new Set that is the explicit cross-product:

model.C = model.A * model.B
model.con = Constraint(model.C, rule=con_rule)

---

Regardless, I still think that you are correct and that removing a component should also remove all implicit sets that the component created.

[tony121psu]

I guess my example was a little oversimplified. In my application, the contents of Set y also change depending on when the constraint is called. Thus re-using the Set causes problems. If I create new Sets for each constraint instance, the workaround works well. However, for now, to be safe, I'm just deleting the implicit sets when deleting the component. Thanks again for your help!

[aoulis]

Hi jsiirola,

I seem to face the issue with tony121psu, and even though I have implemented what you propose in your comment above, I still get the error shown in the screenshot below:

I am attaching a limited example which generates the error and includes the relevant variable (i.e. model.ga).

I would be more than grateful if any of you can help me resolve this issue.

Thank you in advance, aoulis

P.S.: Please let me know if you need any more information on the problem I am trying to solve with my code, or anything else. Limited_example.zip

[jsiirola]

@aoulis: you have basically two options. First, you can just explicitly delete the index when you delete the component. That is,

model.del_component(model.ga)
model.del_component(model.ga_index)

The other option is what I mentioned above and make sure that any component you are going to delete is only indexed by a single Set. Your current definition of ga is:

Var(model.G, model.Timehorizon, initialize = iga, bounds = fga)

Note that that is indexed by two sets G and Timehorizon. If you instead declared a single product set and use it when you declare the component, then Pyomo won't be forced to create the implicit set. For example:

model.GA_index = model.G * model.Timehorizon
if t>0:
    model.del_component(model.ga)
    model.ga = Var(model.GA_index, initialize = iga, bounds = fga)
else:
    model.ga = Var(model.GA_index, initialize = iga, bounds = fga)

[aoulis]

Hi John,

That was very helpful. Indeed it works fine now! (I went for the 2nd option)

Thanks a million, aoulis

[jsiirola]

It occurred to me during a call today that the "right" solution to this issue might have nothing to do with deleting components, but rather to use pyomo.common.modeling.unique_component_name() when auto-generating the implicit indexing sets in the first place.

[whart222]

@jsiirola That's an interesting idea. But would the result be a bunch of set objects that are associated with deleted constraints?

[gseastream]

I would think deleting the implicit sets when you delete the component would make the most sense, since you wouldn't want to end up with garbage, and I would consider it bad to use an implicit set to index another component, since you should have just made an explicit set to index both of them. But there's two caveats: 1) do we currently have a way to know if a component made an implicit index upon construction? 2) even if you delete the implicit set, components that were created with that set still function normally (at least for basic operations):

>>> m.x = Var([1, 2])
>>> m.y = Var(m.x_index)
>>> m.del_component(m.x)
>>> m.del_component(m.x_index)
>>> m.y
<pyomo.core.base.var.IndexedVar object at 0x7f6f6badd7f0>
>>> m.y[2] = 5
>>> m.y[2].value
5
>>> m.y.index_set()
<pyomo.core.base.sets.SimpleSet object at 0x7f6f6bad57d8>
>>> m.y.index_set().name
'x_index'
>>> m.pprint()
1 Var Declarations
    y : Size=2, Index=x_index
        Key : Lower : Value : Upper : Fixed : Stale : Domain
          1 :  None :  None :  None : False :  True :  Reals
          2 :  None :     5 :  None : False : False :  Reals

1 Declarations: y
>>> 'x_index' in dir(m)
False

[jsiirola]

The problem with deleting implicit sets is if other components are also using the implicit set. This can happen if something like a transformation created new components that are based on these components and used the same indexing sets (something like the Connector/Port/Arc expander could do this).

To @whart222's question: yes. This would leave old, potentially unused, Sets on the model. However, that doesn't make the model invalid, and it does resolve the problem of unexpected exceptions.

[gseastream]

In that case, yeah I would think using unique_component_name is the right move, and if the user wants to clean up their implicit sets they have to do that explicitly.

[whart222]

I agree that using unique component names is a reasonable fix to get things working, but the accumulation of temporary sets is an undesirable side-effect of component deletion. That requires more thought for a complete solution.

[qtothec]

Yeah, I'm not a huge fan of having a bunch of unused temporary sets floating around, if only because it pollutes display().

I'm tempted to discourage reuse of implicit sets in some way.

[gseastream]

@qtothec I thought the same until @jsiirola pointed out that the Arc expander uses the same indexing set as the component, even if it's implicit, which makes complete sense to me. The alternative would be creating a copy of the set, but then you get even more sets that way too.

[ghackebeil]

Is there really a need to add implicit sets the model? I understand why they are being created, but couldn't they just be kept alive by the objects that reference them?

[blnicho]

This comment from @mcsoini seems to be missing from the issue for some reason. I’m not sure if this was intentional or a GitHub error but I think the proposed code solution is worth discussing.

From: mcsoini notifications@github.com Sent: Thursday, February 7, 2019 12:29 PM To: Pyomo/pyomo pyomo@noreply.github.com

I added the method below to my model class which inherits from ConcreteModel. The vars loop doesn't feel great, but it does the job for me. Any thoughts on this?

def delete_component(self, comp_name):
        """
        Drop a Pyomo component including index sets.

        A single component object is associated with various index objects.
        Because of this, some looping over the vars is required
        to catch 'em all.

        Args:
            comp_name (str): base name of the model component (variable, etc)
        """
        list_del = []
        for kk in vars(self):
            if ((comp_name == kk)
                or (comp_name + '_index' in kk)
                or (comp_name + '_domain' in kk)):
                list_del.append(kk)
        logger.info('Deleting model components ({}).'.format(', '.join(list_del)))
        for kk in list_del:
            self.del_component(kk)

[mcsoini]

Thanks for pointing this out, @blnicho. Below an updated version. For my purposes this reliably gets rid of all relevant index sets. However, it would break down in case other pyomo components make use of the same sets. Maybe one could add a all=True parameter to the original del_component method and implement something like the code below?

def delete_component(self, comp_name):
        '''
        Drop a component of the pyomo model.

        A single component object is associated with various index objects.
        Because of this, some looping over the vars is required
        to catch 'em all.

        Parameters
        ----------
        comp_name (str): base name of the model component (variable, etc)

        '''

        list_del = [vr for vr in vars(self)
                    if comp_name == vr
                    or vr.startswith(comp_name + '_index')
                    or vr.startswith(comp_name + '_domain')]

        list_del_str = ', '.join(list_del)
        logger.info('Deleting model components ({}).'.format(list_del_str))

        for kk in list_del:
            self.del_component(kk)

[germanmontoya]

Thanks for pointing this out, @blnicho. Below an updated version. For my purposes this reliably gets rid of all relevant index sets. However, it would break down in case other pyomo components make use of the same sets. Maybe one could add a all=True parameter to the original del_component method and implement something like the code below?

def delete_component(self, comp_name):
        '''
        Drop a component of the pyomo model.

        A single component object is associated with various index objects.
        Because of this, some looping over the vars is required
        to catch 'em all.

        Parameters
        ----------
        comp_name (str): base name of the model component (variable, etc)

        '''

        list_del = [vr for vr in vars(self)
                    if comp_name == vr
                    or vr.startswith(comp_name + '_index')
                    or vr.startswith(comp_name + '_domain')]

        list_del_str = ', '.join(list_del)
        logger.info('Deleting model components ({}).'.format(list_del_str))

        for kk in list_del:
            self.del_component(kk)

Hi, I tried your solution but I´m new in Pyomo and Python. I tested many possible configurations of using your function and all of them without success. I have a component called model.C1 and how should I use your function to delete all component items? delete_component(model.C1)? delete_component('model.C1')?

I really appreciate if you can help me.

[mcsoini]

@germanmontoya Note that the function is meant to be used as a method of the ConcreteModel class, or a child thereof. Here is a stand-alone version, with model being a concrete model. Maybe AbstractModels work too, I haven't tried.

def delete_component(model, comp_name):

        list_del = [vr for vr in vars(model)
                    if comp_name == vr
                    or vr.startswith(comp_name + '_index')
                    or vr.startswith(comp_name + '_domain')]

        list_del_str = ', '.join(list_del)
        print('Deleting model components ({}).'.format(list_del_str))

        for kk in list_del:
            model.del_component(kk)

delete_component(model, 'C1')

[germanmontoya]

Thank you! I tested the standalone version. It worked perfectly!

[mrmundt]

@jsiirola - Has this been fully resolved, or are there still steps leftover?

[jsiirola]

@mrmundt: no. The fix will likely require resolution of #2068.