-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
[REVIEW]: PICOS: A Python interface to conic optimization solvers #3915
Comments
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @sfuxy, @marwahaha it looks like you're currently assigned to review this paper 🎉. Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post. ⭐ Important ⭐ If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿 To fix this do the following two things:
For a list of things I can do to help you, just type:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
PDF failed to compile for issue #3915 with the following error:
|
|
@whedon generate pdf from branch joss |
|
@Viech @sfuxy @marwahaha this is the review thread for the paper. All of our communications will happen here from now on. Both reviewers have checklists at the top of this thread with the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines. The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention We aim for reviews to be completed within about 2-4 weeks. Please let me know if any of you require some more time. We can also use Whedon (our bot) to set automatic reminders if you know you'll be away for a known period of time. Please feel free to ping me (@melissawm) if you have any questions/concerns. |
Notifying my co-author @gsagnol. |
👋 @marwahaha, please update us on how your review is going (this is an automated reminder). |
👋 @sfuxy, please update us on how your review is going (this is an automated reminder). |
Hello, folks! Just as a reminder, if you have any questions or need to update the status of the review, please do so here. Thanks! |
Very nice library! Some comments below:
Aside from these comments, I approve this submission. |
Hello @marwahaha, thank you for your review and the kind words! I'll answer your points now and will try to find time to work on the changes next week.
Let me first explain what happens here in detail: PICOS variables are a special kind of an affine expression. Internally, any (multidimensional) variable object is associated with a numeric vector whose entries correspond to the scalar variables handled by solvers, so that the numeric result reported by the solver can be written directly to those vectors. Every entry of a PICOS matrix expression (including matrix variables) is thus stored as a linear combination of any number of such scalar variables plus a constant term, or in other words each entry is an affine expression of those variables. Therefor the lower left entry of a two-by-two variable object Now, the main difference between variable objects and other expressions is that you can directly assign a numeric value to a variable. For other expressions, including general affine ones, such an assignment would already amount to solving an optimization problem. Note that recent versions of PICOS do allow you to assign values to arbitrary affine expression as this only amounts to solving a system of linear equations, but other expressions can only be valued by valuing the variables that appear in them. Now, this is of course a bit much to include in the tutorial, so what part do you think is important to mention? Is it sufficient to explain that "affine = linear + constant offset" or should the reader be made aware of any of the internals?
I admit that most of our examples are a bit clunky and not suited for a quick test. Since their code listings are long, I would not want to present them without some intertext or display of intermediate outputs. However, I can introduce a section of "Simple examples" at the top where all code blocks can be copy-pasted individually in a console to reproduce. Would that be sufficient to you?
That is not easy to delimit as apart from quantum information we don't really address a particular application or scientific field but allow an entire class of numerical algorithms to be implemented. These are of practical interest to fields as diverse as operations research, finance (e.g. portfolio optimization), physics, math (computing counter examples), machine learning, and all sorts of engineering (I've personally used PICOS to optimize a Minecraft farm!). In the paper we wrote:
I can certainly put something like that also in the online introduction, although this is of course a rather vague answer to your question.
The API Reference documents every public class, method, and function, including mathematical details and additional examples (see Ellipsoid for an example of both). In particular, this page lists everything available in the Having an application example for every kind of task that PICOS is up to would be great but is not feasible for us as we maintain the software alongside our current research. What particular methods do you feel are missing from the tutorial or examples?
(Distributionally) robust optimization deals with problems where a cautious decision is required that accounts for uncertainty in the data that you are analyzing. For instance, in finance you could look at the past performance of assets and assume that they will behave similarly in the future, modeled in terms of an estimated probability distribution over their returns. Now if you use plain optimization (more precisely stochastic programming) to find an investement strategy, you are in a mathematically strict sense guaranteed to be disappointed by the results as the method has an intrinsic bias towards overly positive estimates. This is known as the optimizer's curse and DRO is a method to avoid it. We mention this in the paper as one feature where we are currently ahead of the competition. As the competition is pretty tough and all of our tools have been around for a decade, such a we-have-it-but-you-don't feature will always be somewhat niche and its purpose not immediately clear to a general audience. In a paper as short as this one, I can hardly include an explanation of these two models (RO and DRO are related but different) beyond of what there already is, that is mentioning that they deal with uncertainty and giving a reference (which happens to be about PICOS). Instead, I propose adding an RO example to the documentation. Note that concise DRO examples are hard to craft (some of the techniques are from 2020 and don't even have a known application yet) and would overstrain the automatic validation that has only open source solvers at its disposal. |
|
Thank you for the clarification!
I added the following to the Tutorial:
I've added a Quick examples section with three new short examples (least norm, robust optimization, integer programming) that are fully contained in independent listings, including all imports and a shebang line. I've further added the following to the start of the Examples page:
(All examples are CI/CD-validated, so even those in multiple blocks can be reproduced by executing them in order.)
I've changed the first sentence of the README (Introduction in the docs) to:
All public (non-underscore) functions, methods and properties and every Thus, you must be referring to special/magic methods (e.g. I now moved the docstrings of these "mathematical" special methods to the
There is not much I can do about this but I also find it sensible as it introduces little redundancy. Note that with every expression you can follow the link at the top to its base class until you arrive at a parent class where the method is documented. Apart from expressions, I've now documented relevant special methods for
Pyomo has two third-party RO extensions that I know of (PyROS, ROmodel), CVXPY does not seem to have one yet. For DRO I think we are currently the only general purpose interface with direct support, though there are certainly specialized Python interfaces for both techniques that have a more narrow scope than PICOS/Pyomo/CVXPY. |
This all looks great to me! Thanks. |
Thank you for taking the time to review! |
Dear @sfuxy, could you try to complete your review this month? There is currently at least one person looking to cite PICOS in a paper. |
(I've send them a mail.) |
Thanks, @Viech - @sfuxy gentle ping on this :) |
I did not get a response by mail either. As this has now been open for over two months, could you assign another reviewer @melissawm? |
I think that sounds reasonable @Viech - do you have suggestions? |
Sure @melissawm, there are a few candidates in the list with Python as a preferred language and who seem to be particularly interested in mathematical optimization or OR. Ordered by newest list entry first:
I suggest to ask in this order, skipping anyone who may be actively involved in the software. At this point it woud be nice if the reviewer can start soon. |
@whedon set v2.4 as version |
OK. v2.4 is the version. |
@whedon recommend accept |
I'm sorry human, I don't understand that. You can see what commands I support by typing:
|
@whedon recommend-accept |
|
PDF failed to compile for issue #3915 with the following error:
|
@whedon recommend-accept from branch joss |
|
|
👋 @openjournals/joss-eics, this paper is ready to be accepted and published. Check final proof 👉 openjournals/joss-papers#2968 If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#2968, then you can now move forward with accepting the submission by compiling again with the flag
|
Now that the links on the sidebar are supposed to work, I checked them and found that your URL |
@whedon recommend-accept from branch joss |
|
|
👋 @openjournals/joss-eics, this paper is ready to be accepted and published. Check final proof 👉 openjournals/joss-papers#2969 If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#2969, then you can now move forward with accepting the submission by compiling again with the flag
|
Should be fixed now - thanks @Viech ! |
LGTM. Thank you! |
@whedon accept deposit=true from branch joss draft looks good to me! |
|
🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦 |
🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨 Here's what you must now do:
Any issues? Notify your editorial technical team... |
Congratulations to @Viech (Maximilian Stahlberg) and co-author!! And thanks to @marwahaha and @GuillaumeDerval for reviewing, and @melissawm for editing! |
🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉 If you would like to include a link to your paper from your README use the following code snippets:
This is how it will look in your documentation: We need your help! Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:
|
Awesome! Thank you very much @melissawm, @marwahaha and @GuillaumeDerval for your time and effort! Also to @danielskatz for proofreading and pushing the button! ❤️ |
Submitting author: @Viech (Maximilian Stahlberg)
Repository: https://gitlab.com/picos-api/picos
Version: v2.4
Editor: @melissawm
Reviewers: @marwahaha, @GuillaumeDerval
Archive: 10.5281/zenodo.6053359
Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.
Status
Status badge code:
Reviewers and authors:
Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)
Reviewer instructions & questions
@sfuxy & @marwahaha, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @melissawm know.
✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨
Review checklist for @sfuxy
✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
Review checklist for @marwahaha
✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
Review checklist for @GuillaumeDerval
✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
The text was updated successfully, but these errors were encountered: