Flexdims/4

[chinasaur]

See abstract_dim for main changes. I implemented the arithmetic operators that I needed to get the CBP example to work, but I haven't tested other examples, so there are probably things missing. E.g. AD.mul assumes the other is also an AD, but may need to be willing to handle an int. Most of the other operators handle ints.

Changes to codegens and encoders was mostly just changing %d to %s in a bunch of places.

The ugly part was trying to get the abstract dim variable names properly wrapped (dims.m for Matlab, dims['m'] for Python, dims->m for C). It might have been easier if I hadn't already hacked out the old dimension_dictionary stuff, but still I think the new way is cleaner. Codegens now must implement abstract base method abstractdim_rewriter. When shape.size() is called in Codegen, this rewriting function is passed in to translate m to dims.m, etc.

Matlab codegen gets correct optval. C codegen compiles and Python looks good, but I haven't tested those. All codegen prob2socp now take a dims argument as second argument; I didn't add any logic to test whether the dims argument is really needed, i.e. whether there are any dims that aren't set to straight integers.

A future possibility would be to pull dims out of the function arguments and use the size of one of the params to determine dims. Requires walking through the params and finding one whose shape reflects each of the abstract dims.

I changed the name of the dims.l dims.q dims.s variable to cones.l cones.q cones.s to avoid clashing with the input dims variable. When 'cones' is loaded into 'data' at the end though, it goes in as 'dims' to be compatible with ECOS.

Shape no longer pays much attention to whether it is "instantiated" as the rest of the code can now handle concrete int dims or abstract dims. But I left the instantiated logic in there as it was used by the future slice method and I didn't think through how that would play out.

I think that sums it up; hope it's not a bear to merge with your changes!

[echu]

Sweet; I've thought about inferring dims from the parameters, but it's not the case you'll always be able to:

dimension n variable x(n) minimize sum(square(x))

There are no parameters there.

Anyway, we'll put it together. I'm working slowly on my side, so feel free to submit more changes if you run into bugs, etc.

Eric

On Sun, Sep 15, 2013 at 9:08 PM, Peter H. Li notifications@github.comwrote:

See abstract_dim for main changes. I implemented the arithmetic operators that I needed to get the CBP example to work, but I haven't tested other examples, so there are probably things missing. E.g. AD.mul assumes the other is also an AD, but may need to be willing to handle an int. Most of the other operators handle ints.

Changes to codegens and encoders was mostly just changing %d to %s in a bunch of places.

The ugly part was trying to get the abstract dim variable names properly wrapped (dims.m for Matlab, dims['m'] for Python, dims->m for C). It might have been easier if I hadn't already hacked out the old dimension_dictionary stuff, but still I think the new way is cleaner. Codegen's now must implement abstract base method abstractdim_rewriter. When shape.size() is called in Codegen, this rewriting function is passed in to translate m to dims.m, etc.

Matlab codegen gets correct optval. C codegen compiles and Python looks good, but I haven't tested those. All codegen prob2socp now take a dims argument as second argument; I didn't add any logic to test whether the dims argument is really needed, i.e. whether there are any dims that aren't set to straight integers.

A future possibility would be to pull dims out of the function arguments and use the size of one of the params to determine dims. Requires walking through the params and finding one whose shape reflects each of the abstract dims.

I changed the name of the dims.l dims.q dims.s variable to cones.l cones.q cones.s to avoid clashing with the input dims variable. When 'cones' is loaded into 'data' at the end though, it goes in as 'dims' to be compatible with ECOS.

Shape no longer pays much attention to whether it is "instantiated" as the rest of the code can now handle concrete int dims or abstract dims. But I left the instantiated logic in there as it was used by the future slice method and I didn't think through how that would play out.

I think that sums it up; hope it's not a bear to merge with your changes!

You can merge this Pull Request by running

git pull https://github.com/chinasaur/qcml flexdims/4

Or view, comment on, or merge it at:

https://github.com/cvxgrp/qcml/pull/30 Commit Summary

• QCML.dims now delegates to Program.dimensions
• Playing around with abstract_dim
• Merge branch 'master' of https://github.com/cvxgrp/qcml into flexdims/3
• Trying to use AbstractDims but with concrete dims
• Added eq to AbstractDim to support expr simplifications
• Work in progress: abstract dim generates Matlab code!
• Adapt CBP example for flexdims
• Bugfixes for AbstractDim
• Raw code output back in
• AbstractDim printing cleanup
• Docu
• Now allows abstract dim variable name to be rewritten
• Switch back to m,n for CBP example
• In progress gettings dims argument into prob2socp
• Geting C codegen setup with flexdims argument
• Didn't mean to include this
• Finish changing back to m,n for CBP example

File Changes

• M examples/cbp.pyhttps://github.com/cvxgrp/qcml/pull/30/files#diff-0(8)
• M src/ast/problems/program.pyhttps://github.com/cvxgrp/qcml/pull/30/files#diff-1(4)
• M src/codegens/C/codegen.pyhttps://github.com/cvxgrp/qcml/pull/30/files#diff-2(33)
• M src/codegens/C/stuff_template.hhttps://github.com/cvxgrp/qcml/pull/30/files#diff-3(7)
• M src/codegens/base_codegen.pyhttps://github.com/cvxgrp/qcml/pull/30/files#diff-4(33)
• M src/codegens/matlab/codegen.pyhttps://github.com/cvxgrp/qcml/pull/30/files#diff-5(23)
• M src/codegens/python/codegen.pyhttps://github.com/cvxgrp/qcml/pull/30/files#diff-6(12)
• M src/codes/encoders/c_encoder.pyhttps://github.com/cvxgrp/qcml/pull/30/files#diff-7(12)
• M src/codes/encoders/matlab_encoder.pyhttps://github.com/cvxgrp/qcml/pull/30/files#diff-8(12)
• M src/codes/encoders/python_encoder.pyhttps://github.com/cvxgrp/qcml/pull/30/files#diff-9(6)
• A src/properties/abstract_dim.pyhttps://github.com/cvxgrp/qcml/pull/30/files#diff-10(154)
• M src/properties/shape.pyhttps://github.com/cvxgrp/qcml/pull/30/files#diff-11(22)
• M src/qc_lang.pyhttps://github.com/cvxgrp/qcml/pull/30/files#diff-12(18)

Patch Links:

• https://github.com/cvxgrp/qcml/pull/30.patch
• https://github.com/cvxgrp/qcml/pull/30.diff

[chinasaur]

Yeah, there would be cases where it wouldn't work, but I think still a nice feature and it would work in a lot of cases.

Just added default dims={} and tested the portfolio example with concrete dims; it reaches an optimum; how to test whether optimum is correct?

[chinasaur]

I see I broke a bunch of tests; I'll start cleaning those up.

[echu]

Nice; apparently having the "auto-tester" is kind of nice. :)

As for the "auto-detect" you might be right; another thing we could do is just have it figure it out automatically, e.g.,

dims['m'], dims['n'] = A.shape

And for the ones it can't figure out, it can issue a warning or something.

[chinasaur]

Now that I know how to run the tests on Linux and Mac I'll try to be better about that :). Autotester is nice though.

Tests are passing locally now; hopefully travis agrees.

[echu]

Looks good; you made the modifications in all the right places. I may reorganize a little on my end, but I'm likely not to commit anything for a little bit until I sort out this expression madness. Thanks! Let me know how it works on your CBP problem.

[chinasaur]

Cool; I'll be testing a lot soon! I think I'll back off on dimensions_dictionary versus abstractdim_rewriter; it may be simpler to go back to the dimensions_dictionary approach. Let me know if you think that's worth reverting. It works this way though.

I could write some tests to cover AbstractDim; would be clearer than trying to write docs to describe what it does and doesn't do...

Other thoughts: maybe it's simpler to put the dims inputs inside of params?

I think I would also like to have the top comment in codegen output include more information such as the original parsed text as well as the canonicalized form. Or have the option to include this.

Also, since I recently watched Stephen's lecture on perturbation sensitivity, should we try to help the user get back out the dual optimal variables as well? Those should be easy to extract, right?

[chinasaur]

Oh, I also thought, for the Matlab and C codegens at least, we should provide a third wrapper function that will take the params inputs, run prob2socp, call ECOS as appropriate, run socp2prob, and return x and optval (and perhaps lambda/nu). In the Matlab case, would be convenient to have these functions print out with the wrapper function at the top, that way prob2socp and socp2prob can be nested functions in the same file if the user wants to do it that way; then the QCML output could conceivably be dumped directly to an .m file and be ready to run.

[echu]

On Mon, Sep 16, 2013 at 8:21 PM, Peter H. Li notifications@github.comwrote:

Cool; I'll be testing a lot soon! I think I'll back off on dimensions_dictionary versus abstractdim_rewriter; it may be simpler to go back to the dimensions_dictionary approach. Let me know if you think that's worth reverting. It works this way though.

I could write some tests to cover AbstractDim; would be clearer than trying to write docs to describe what it does and doesn't do...

I don't have a preference on this. To be honest, something that's simple and "non-hacky" is the approach I'd take (even if it's not necessarily the fastest). Ideally, we write a single

AbstractAlgebra

class that we can subclass for expressions and dimensions (dimensions having ints, but expressions having doubles and nonlinear functions). At the moment, though, it looks like they'll have to be two separate things.

Other thoughts: maybe it's simpler to put the dims inputs inside of params?

Possibly. It's a terminology thing, right? The optimization "code" maps numeric parameters into solutions. The dims sort of "specialize" the optimization code. So in some sense, dims are "like" template arguments (in C++ terminology) and should remain a separate argument. On the other hand, if you wanted to argue that the "code" maps the parameters and dims into solutions, I can see that also.

If we think more long term and have dimension inference built in to the code, than dims should definitely be separate (and used when the dims can't be inferred). So... we should keep them separate.

I think I would also like to have the top comment in codegen output include more information such as the original parsed text as well as the canonicalized form. Or have the option to include this.

I agree. :) I don't even think it should be an option. It should just do it. I had planned to revisit this once everything worked; that time is now. Feel free to do this!

Also, since I recently watched Stephen's lecture on perturbation sensitivity, should we try to help the user get back out the dual optimal variables as well? Those should be easy to extract, right?

Yes! They are easy to extract. ECOS outputs x,s,z,y; I forget exactly, but z and y correspond to the dual variables. So you could easily just index into z and y to get what you want.

— Reply to this email directly or view it on GitHubhttps://github.com/cvxgrp/qcml/pull/30#issuecomment-24561601 .

[echu]

Agreed.

Also, we should add a ".save" to the qc_lang file. In Python, it can output a module with the prob2socp, socp2prob functions defined, along with a third function that calls ECOS. In Matlab, it can output three .m files (as you describe). In C, it can output the three functions you suggested.

Currently, the ".save" functionality is implemented in the C codegen, and we should probably just separate it out. Maybe call it a qc_io or something.

On Mon, Sep 16, 2013 at 8:32 PM, Peter H. Li notifications@github.comwrote:

Oh, I also thought, for the Matlab and C codegens at least, we should provide a third wrapper function that will take the params inputs, run prob2socp, call ECOS as appropriate, run socp2prob, and return x and optval (and perhaps lambda/nu). In the Matlab case, would be convenient to have these functions print out with the wrapper function at the top, that way prob2socp and socp2prob can be nested functions in the same file if the user wants to do it that way; then the QCML output could conceivably be dumped directly to an .m file and be ready to run.

— Reply to this email directly or view it on GitHubhttps://github.com/cvxgrp/qcml/pull/30#issuecomment-24561931 .