Skip to content
This repository has been archived by the owner on Nov 19, 2020. It is now read-only.

A few updates to optimization constraints #821

Closed
4 of 7 tasks
CatchemAL opened this issue Aug 29, 2017 · 3 comments
Closed
4 of 7 tasks

A few updates to optimization constraints #821

CatchemAL opened this issue Aug 29, 2017 · 3 comments

Comments

@CatchemAL
Copy link
Collaborator

CatchemAL commented Aug 29, 2017

Hi @cesarsouza,

I had a few thoughts on some of the constraints that exist in the library and I was wondering if you'd be interested in these changes?

Changes

  • Add double Tolerance { get; } to the IConstraint interface
  • Add bool IsViolated(double[] input) as an extension method on IConstraint (or add it to the interface)
  • On IConstraint, promote Func<double[], double> Function { get; } to double Function(double[] x)
  • Similarly, promote Func<double[], double[]> Gradient { get; } to double[] Gradient(double[] x)
  • Separate the business logic from the parsing logic by moving the parsing logic into ExpressionParser or some similar class.
  • Tidy up constructors in the NonlinearConstraint and LinearConstraint
  • Add support for void Gradient(double[] x, double[] grad) nonlinear constraints

Why?

  • Tolerance: would make Augmented Lagrangian to support linear constraints #820 easier.
  • IsViolated: Useful. I would propose an extension method so all IConstraints get it for free. I probably wouldn't add it to the interface as it feels a bit mean to make implementers of IConstraint implement this over and over.
  • Promote: I really feel it is much, much more intuitive to expose methods rather than readonly access to a Func<double[], double>. To me, the API is far clearer and it stops the intellisense bringing up obscure properties when filling in constraint properties. I'm aware that there is some dynamism with Expressions being compiled but this can all be handled without any issue. (Proposed code snippet below)
  • Tidy: Some of the constructors request unnecessary objects (e.g. the ObjectiveFunction when they really just want to know the NumberOfVariables). Some don't request enough to work properly (e.g. public LinearConstraint(int numberOfVariables)) making it hard for the uninitiated to know what properties need to be set. I noticed someone raised an issue yesterday requesting help. Perhaps I could add some constructors that specify the exact requirements, fully document those and obsolete some others?
  • Gradient: I am aware that one could pass in some sort of method that could internally cache the gradient but it would be nice if it was supported out-the-box. Much faster for problems with lots of constraints if they don't have to allocate lots of gradient vectors on every iteration.

Let me know if you'd be interested in me adding any of these.

Thanks,
Alex

public class Current
{
    public Current(string some, int inputs)
    {
        // Save the inputs....

        // Assign
        this.Function = compute;
    }

    public Func<double[], double> Function { get; private set; }

    private double compute(double[] x)
    {
        // some complicated calc that uses some inputs.
        return 0;
    }
}

public class Proposed
{
    public Proposed(string some, int inputs)
    {
        // Save the inputs....
    }

    public double Function(double[] x)
    {
        // some complicated calc. that uses some inputs
        return 0;
    }
}
@cesarsouza
Copy link
Member

cesarsouza commented Aug 29, 2017

Hi @AlexJCross,

Thanks a lot for all the nice suggestions! I will comment on them below:

  1. Add double Tolerance { get; } to the IConstraint interface
    I think this is a really good idea, but as long as all optimization methods are updated to either take the value of this property into consideration, or throw exceptions when they can't. I am not sure how much effort would be necessary for this. It would also be better if those checks could be made in a single place so that we wouldn't forget to perform them when we are implementing a new constrained optimization algorithm.

  2. Add bool IsViolated(double[] input) as an extension method on IConstraint (or add it to the interface)
    Again, really good idea! I also think that adding it as an extension method would be better for the same reason you mention, and I guess there would be no situation where a constraint would need to override this implementation, thus making an extension method ideal!

  3. On IConstraint, promote Func<double[], double> Function { get; } to double Function(double[] x)
    Similarly, promote Func<double[], double[]> Gradient { get; } to double[] Gradient(double[] x)
    Please see 5 below!

  4. Separate the business logic from the parsing logic by moving the parsing logic into ExpressionParser or some similar class.
    👍

  5. Tidy up constructors in the NonlinearConstraint and LinearConstraint
    Suggestions 3 and 5 are very nice, and I think it would be really nice to incorporate them. I was a little concerned about keeping compatibility, but it is true that since they are lambdas with get-only accessors it would not be that much of a breaking change, and they would allow for the implementation of item 6 below, which wouldn't have been possible before since it would not have been possible to overload a property. Regarding constraints that can actually accept lambdas, such as NonlinearConstraint, I understand that they would still be able to accept lambda functions in their constructors and have them called by the Function / Gradient methods, right? We would be adding a new layer of indirection here, but I guess it should be fine given the improvement in usability!

  6. Add support for void Gradient(double[] x, double[] grad) nonlinear constraints
    👍

Thanks again,
Cesar

@CatchemAL
Copy link
Collaborator Author

Hi @cesarsouza

It would also be better if those checks could be made in a single place so that we wouldn't forget to perform them when we are implementing a new constrained optimization algorithm.

I hadn't thought about this but I'll check it out - sounds v sensible.

I understand that they would still be able to accept lambda functions in their constructors and have them called by the Function / Gradient methods, right? We would be adding a new layer of indirection here...

Yep - that's exactly what I had in mind. An extra layer of indirection in the NonLinearConstraint and one less layer of indirection in the LinearConstraint.

Best,
Alex

@CatchemAL CatchemAL self-assigned this Aug 30, 2017
CatchemAL added a commit that referenced this issue Oct 11, 2017
 - Adding Tolerance to the interface of IConstraint
 - Add GetViolation and IsViolated as extension methods to IConstraint (removing duplicated code)

GH-821:
 - Changing all references from NonlinearConstraint to IConstraint in AugmentedLagrangian.cs.
 - Updating the documentation to give an example of using LinearConstraints in the AugmentedLagrangian
CatchemAL added a commit that referenced this issue Oct 11, 2017
 - Adding Tolerance to the interface of IConstraint
 - Add GetViolation and IsViolated as extension methods to IConstraint (removing duplicated code)

GH-820:
 - Changing all references from NonlinearConstraint to IConstraint in AugmentedLagrangian.cs.
 - Updating the documentation to give an example of using LinearConstraints in the AugmentedLagrangian
@cesarsouza cesarsouza added this to the 3.8 milestone Oct 22, 2017
@cesarsouza
Copy link
Member

Added in 3.8.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants