JOSS review

[odunbar]

Purpose

Issues found during covering the checklist: openjournals/joss-reviews#6226
Please address these either by return message in this thread, or by adding pull-requests/commits tagging these issues.
I'll check of these points when satisfied!

Overall Impression

I see this as a usable, light, and compact tool in Julia. Certainly it is worthy of being a registered package. However, for a JOSS paper, more evidence of package usefulness in application is required. I believe we could get a JOSS publication here, but there needs to be some more proof, in the form of examples/performance tests of what is implemented, and the software paper should state claims reflecting what is actually present in the codebase.

General Checks

• Contribution and authorship: The lead has made all the code contributions. There is currently no record of the other authors (open) contributions to the code-base, in cited articles, or in other uses of the code-base. Could the other authors' contributions be stated in a response here?
• Substantial scholarly effort: At first glance, the age of the software appears to be a few months, with few commits, there appears to be one developer, and no published materials for this software. So as it stands, at first glance, this code does not meet the minimum requirement for JOSS.
  However A cursory look at the history of this package is that all source code was copied from somewhere into this repository? Only a couple of commits relate to the source code, most of them appear to be setting up the GH actions/documentation build. I also am not sure why the initial package release started at v0.8.0? Perhaps the author(s) could shed some light in a response here too?

Functionality

• Have the functional claims of the software been confirmed? It is hard to verify whether the code can achieves what was set out. There are minimal claims, and only one toy example (though it is easily reproducible), without validation of the result. I outline some possibilities of examples, a selection of which, if provided and documented, would provide trust that the tool is working.
  (1) Mathematical analysis: i.e Is there any analytical statements exist of proof of convergence/rate of convergence and does the code acheive this on test problems. Or other things, such as when will one observe dependence of solution on the initial point?
  (2) how it could solve a "tough" problem - e.g. a toy example with some unpleasant nonlinearities/poorly conditioned Hessians for r_i or c_i
  (3) Showcase a test problem from the Hydro-Quebec application.
  (4) A guides to the user for when the code is more/less performant or plots of how it scales with increasing numbers of equations/constraints
• Restrictive typing: the code cannot take anything except Float64. could this be extended to work with any precision (<:AbstractFloat). Likewise, the provided functions appear to need to take in and return specific types which is not clear. For examples, the Jacobians appear to need to take in Vector{Float64} and return Matrix{Float64} (if i interpret the example correctly), could such restrictions be made more clear for users.
• Diagnostics Can one print diagnostics after solving (e.g. print the information that would be output from solving with silent=false)? This would allow users to consult a solved model to query the convergence without needing to re-solve with the silent flag. #4

Documentation

• Index page: The Contents self-references, maybe remove the first few items. or remove altogether. As the contents appear on the left column already
• Index page: Could here or elsewhere summarize how the Active-set and Gauss-Newton methods work (i.e. what is going on under the hood here), for someone familiar with basic optimization knowledge.
• Index page: Typos e.g. "litterature" in index.md
• No Community Guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support
• The automated tests run two examples, there are no unit tests for non-exported functions
• Usage page: Typo x \in R^n not given
• Usage page: Is the norm always the Euclidean norm?
• Usage page: Typos e.g. "optionnal" "jacobian matrices computations".
• Usage page: the docstring for status could be expanded. what are all the options and what do they mean?

Software paper

• Summary, define "multifunction"? is this more than just a function r:R^n -> R^m ?
• Statement of need There are some "quoted" good performance, in the paper, but there are no references or examples to actually support the statements. One should remove any references to these unpublished or un-evidenced claims, or present evidence for them.
• What about lpopt.jl? This I think solves all the constrained problems too?
• There are also tools that use autodiff to aid computation rather than having to provide Jacobians in this field, are there any plans for such functionality here?
• Please state the latex formulation of problem 65 before you write the code solver, so it is clear what is going on.
• Please state how to obtain the solution from model as it is not simply returned by solve!(model)
• Are there any published materials for the Fortran 77 code when used for Hydro-Quebec? If so please reference these too.

Misc

• You can choose to ignore this comment; but there is a git-wide push for using main not master, (it is a simple change for developers)

From review corrections

• Software paper, typo with [ ] in autodifferentiation reference
• Software paper, could you add to the final example that calling solution(model) gives the user the solution
• Chain-Rosenbrock example, as the purpose of this example is solving with higher dimensions. Could you provide the scaling in time for different n that users may encounter. I think perhaps a table with just time to solution for n=10,n=100, n=1000, (...n=10^4 if that's possible...) over 10 repeats would be great.

[pierre-borie]

First of all, please accept my apologies for taking this much time to respond your review comments.

To answer your General Checks remarks, the other authors did not actually contribute to the code per se but they were important stakeholders in the conception of this package which is why we thought relevant to refer them as authors. To bring some details, Alain Marcotte helped me to carry out the comparison tests between Julia and Fortran and the other two supervised the conduct of this project.
As you already kind of guessed, the code was first developed on a local repository and then transfered to my lab GitHub when we published it on the Julia package manager, which explains the small number of commits and the current versioning. I hope this clarifies the background of the package!

Thanks a lot for your suggestions of improvement. I started to bring corrections and additions to the package source code and documentation in accordance with your remarks and will come back to you once I finished completing them as you suggested.

I am, once again, sorry for my response delay.

[odunbar]

No problem! Thanks for onboarding the feedback

[pierre-borie]

I modified the documentation by adding a description of the method and correcting typos. I also changed the status docstring eventhough it is not visible in the documentation page yet.
I am currently working with Hydro-Québec to provide an industrial use of our package.

[pierre-borie]

Hi! I pushed a revised version of the paper. A brief description of the method and its theoretical guarantees have been added. There is also a paragraph about an Hydro-Québec application.

[odunbar]

Thanks @pierre-borie

I think I have ticked off everything that I can clearly see have been completed. I also understand that the HQ example is tough to use due to the proprietary parts.

• Please point me to the other items if I have missed them.
• I have a couple small additional items based on your corrections (see bottom of my issue above)

[pierre-borie]

@odunbar - Thanks for your positive feedback on the new version. From what I can see, there is just the "diagnostics" issue that has not been checked though the corresponding functionality has been added but it might be my fault. I wanted to mention this issue specifically but did wrong somewhere and tried to backtrack.

I corrected the paper accordingly to your last items and added a table for Chained Rosenbrock problem showing time for different dimensions on the dev version of the documentation.

For now, I am waiting for the other reviewer's comments before submiting a revised version.

[odunbar]

OK great! I like the table, thank you for the response and I'll take your word for it on the software paper typo's