Refactor solve_rotation_ap and bugfix

[carterbox]

This pull request has two components:

1. Refactored solve_rotation_ap in order to conform to left multiplication of rotation matrices.

2. numpy was throwing an error inside region_diff because of size mismatch using vstack. I've made a patch, but I am not sure that it captures the intent of the original.

Please see commit messages for more detailed explanation.

[carterbox]

Apparently, the bug was already located. I can remove that commit. However, the numpy documentation states that we should prefer np.concatenate over np.hstack.

[johnyf]

Thanks for the changes. Indeed, the bug was fixed, please see comments here. I will merge the refactoring changes, and consider using np.concatenate.

[johnyf]

I manually cherry-picked the changes of e45750582a53bb9b41274e5445faade5a920851e as fa3767abf05b2246cc51dde300b6bbd7b7dcf1b6, in order to resolve conflicts (mainly due to recent style changes). As already observed [above](), the changes of 4db7df5e49b9ba1e9b7c2a717ecbe3161ed3452e have been superseded by those in fcf751a04ca0b10761163b222c6e62afbd1dcbdc.

Some comments:

• please prefer passing keyword arguments to the method str.format, because they are more readable, as recommended also here.
• I support logging. However, it is worth considering where logging calls are placed, and avoid inserting them in areas that are frequently called, because they can substantially increase the runtime of a Python script (noticeably). I mention this because polytope has recursive functions within Python that are called very frequently during computation.
• Disabling logging by changing logging level does not avoid the cost of str.format, which can be substantial. There are several aspects to this problem, and the general solution is to pass positional arguments to logging calls, as described here. However, I do not support sacrificing readability for performance (i.e., using % formatting syntax and positional arguments to logging calls). Instead, I think that it works better to either:
  1. try to avoid logging calls in busy areas when not essential
  2. try to place plain string messages in necessary logging calls located in busy areas
  3. defer logging of data (which usually benefits from formatting) to the end of functions that aren't frequently called, or avoid it, or comment it out
  4. guard logging or sanity checking calls that involve expensive computation using Logger.getEffectiveLevel (example)

A much better logging solution seems to be to use the package logbook, as suggested here. I just started considering using logbook in my code. It is Pythonic (unlike the standard library's logging), and it does support keyword argument formatting for strings, as in log.debug('x = {x}', x=1). I do not know how performance compares to the standard library, but I would expect it has been taken into consideration. In particular:

"Logbook was designed to be fast and with modern Python features in mind. For example, it uses context managers to handle the stack of handlers as well as new-style string formatting for all of the core log calls."

and

"If properly configured, Logbook’s logging calls will be very cheap and provide a great performance improvement over an equivalent configuration of the standard library’s logging module."

and

"Here a list of things you can expect in upcoming versions: c implementation of the internal stack management and record dispatching for higher performance."

Further comments:

• "u is now" in logging message: "now" can be omitted, "is" can be "=",
• I let hstack be used, instead of concatenate, because of the comments https://github.com/tulip-control/polytope/pull/44/commits/4db7df5e49b9ba1e9b7c2a717ecbe3161ed3452e#r115130564.

[carterbox]

@johnyf Thanks for that those interesting notes about logging. I didn't realize that it was so expensive. Also, since I was mainly using all those logging statements for development purposes, they can be commented out or removed from the release version.

[johnyf]

I no longer consider logbook as a candidate replacement for the standard library's logging (mentioned above). A detailed commentary by the maintainer of logging can be found here.