Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding (parameterized) linear programming dual transformation! #3402

Merged
merged 81 commits into from
Nov 15, 2024

Conversation

emma58
Copy link
Contributor

@emma58 emma58 commented Nov 5, 2024

Fixes the complaints of several reasonable people

Summary/Motivation:

This PR adds a transformation in core to take the dual of a linear program. For the purposes of supporting bilevel programming, it also allows for this dual to be "parameterized" by certain variables--that is, the user can specify a list of Vars that are treated as data for the purposes of taking the dual (e.g., if they are the outer variables of a bilevel program with an LP inner problem).

Changes proposed in this PR:

  • Adds core.lp_dual transformation.
  • Adds a parameterized standard form writer to get the A, b, and c matrices for taking the dual.
  • Adds a very vanilla Python implementation of CSR and CSC matrices for use in the above--we can't use scipy because we need to allow Pyomo expressions in sparse matrix representations

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the BSD license.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

…that assume that the data is numeric. Creating my own csr and csc classes that are almost complete
…use the correct converstion to a CSC matrix depending on the circumstances, testing the conversion to nonnegative vars
Copy link
Contributor Author

@emma58 emma58 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding comments from an in-person review with @jsiirola

pyomo/util/var_list_domain.py Outdated Show resolved Hide resolved
pyomo/core/plugins/transform/lp_dual.py Outdated Show resolved Hide resolved
pyomo/core/plugins/transform/lp_dual.py Outdated Show resolved Hide resolved
pyomo/core/plugins/transform/lp_dual.py Outdated Show resolved Hide resolved
pyomo/core/plugins/transform/lp_dual.py Outdated Show resolved Hide resolved
@emma58
Copy link
Contributor Author

emma58 commented Nov 11, 2024

@mrmundt @jsiirola Assuming tests pass, this is ready for review again.

Comment on lines 33 to 51
def __call__(self, x):
if hasattr(x, 'ctype') and x.ctype is self._ctype:
if not x.is_indexed():
return ComponentSet([x])
ans = ComponentSet()
for j in x.index_set():
ans.add(x[j])
return ans
elif hasattr(x, '__iter__'):
ans = ComponentSet()
for i in x:
ans.update(self(i))
return ans
else:
_ctype_name = str(self._ctype)
raise ValueError(
f"Expected {_ctype_name} or iterable of "
f"{_ctype_name}s.\n\tReceived {type(x)}"
)
Copy link
Member

@jsiirola jsiirola Nov 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some issues with this implementation:

  1. it doesn't actually return a list. ;)
  2. it doesn't actually validate the ctype for list-like inputs

Maybe the following would be a more robust alternative:

def __init__(self, ctype):
    if isinstance(ctype, Sequence):
        self._ctypes = set(ctype)
    else:
        self._ctypes = set([ctype])

def __call__(self, x):
    return list(self._process(x))

def _process(self, x):
    if hasattr(x, 'ctype'):
        if x.ctype not in self._ctypes:
            raise ValueError(" .... ")
        if x.is_indexed():
            yield from x.values()
        else:
            yield x
    elif isinstance(x, Sequence):
        for y in x:
            yield from self._process(y)
    else:
        raise ValueError(" ... ")

def domain_name(self):
    _names = ', '.join(ct.__name__ for ct in self._ctypes)
    if len(self._ctypes) > 1:
        _names = '[' + _names + ']'
    return f"ComponentDataList({_names})")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually really want it to both return and accept ComponentSets, but just took your suggestions otherwise (and renamed it accordingly)

Copy link
Member

@jsiirola jsiirola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very, very close

return _ParameterizedLinearStandardFormCompiler_impl(config).write(model)


class _SparseMatrixBase(object):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some day (not today) we should refactor the sparse sets into pyomo.common

pyomo/util/config_domains.py Outdated Show resolved Hide resolved
pyomo/util/config_domains.py Show resolved Hide resolved
Comment on lines 41 to 43
# Ordering for determinism
_ctypes = sorted([ct.__name__ for ct in self._ctypes])
_names = ', '.join(_ctypes)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to convert the set() to a list: that is unnecessary and converts O(1) lookups into O(n). We should sort the names ... but again, we should only generate the names string if we are going to actually use it (i.e. before raising an exception).

@blnicho blnicho merged commit f520b2a into Pyomo:main Nov 15, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants