Closed whedon closed 2 years ago
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @odow, @jbcaillau it looks like you're currently assigned to review this paper :tada:.
:warning: JOSS reduced service mode :warning:
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.
:star: Important :star:
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:
@whedon commands
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
@whedon generate pdf
Software report (experimental):
github.com/AlDanial/cloc v 1.88 T=0.03 s (1161.1 files/s, 109233.8 lines/s)
-------------------------------------------------------------------------------
Language files blank comment code
-------------------------------------------------------------------------------
Julia 20 261 198 2071
Markdown 6 152 0 627
YAML 6 3 3 153
TOML 4 5 0 76
Bourne Shell 1 4 0 12
TeX 1 0 0 10
-------------------------------------------------------------------------------
SUM: 38 425 201 2949
-------------------------------------------------------------------------------
Statistical information for the repository 'f1301324261094a58bb61737' was
gathered on 2021/12/14.
No commited files with the specified extensions were found.
PDF failed to compile for issue #3991 with the following error:
Can't find any papers to compile :-(
Dear @odow and @jbcaillau
This is the review issue. There are 20 tasks for each reviewer. Whenever an item is solved, you can check them.
You can interact with the other reviewers, the author(s), and me. You don't have to complete your review in one step, so while the authors improve the software and paper, you can continue your review with the other tasks.
Reviewers can also contribute to repository, open new issues in the target repo as well. Please mention this issue in your review in the target repo so we can keep our eyes on what is going on broadside.
Please do not hesitate to ask me anything, any time.
@whedon check references from branch joss-paper-branch
Attempting to check references... from custom branch joss-paper-branch
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.1137/070679557 is OK
- 10.1007/s10589-014-9687-3 is OK
- 10.1007/s10107-004-0559-y is OK
- 10.21105/joss.00615 is OK
- 10.5281/zenodo.3969045 is OK
- 10.1287/ijoc.2021.1067 is OK
- 10.5281/zenodo.3948381 is OK
- 10.5281/zenodo.1188851 is OK
- 10.5281/zenodo.2558627 is OK
- 10.5281/zenodo.822073 is OK
- 10.5281/zenodo.3900668 is OK
- 10.5281/zenodo.2658672 is OK
- 10.1145/992200.992202 is OK
- 10.1109/TPDS.2018.2872064 is OK
- 10.1147/rd.471.0057 is OK
- 10.1145/962437.962438 is OK
- 10.1145/1236463.1236467 is OK
- 10.3390/pr6080106 is OK
- 10.13140/rg.2.1.2846.6803 is OK
- 10.1007/978-1-4419-6935-4_15 is OK
- 10.1007/978-1-4614-3226-5 is OK
- 10.1007/s00158-011-0666-3 is OK
- 10.1038/s41592-019-0686-2 is OK
- 10.1137/141000671 is OK
- 10.1287/ijoc.2014.0623 is OK
- 10.5281/zenodo.3994983 is OK
- 10.5281/zenodo.2629034 is OK
- 10.1007/0-387-30065-1_4 is OK
- 10.1137/15M1020575 is OK
- 10.1287/mnsc.36.5.519 is OK
- 10.5281/zenodo.2655082 is OK
- 10.5281/zenodo.5056629 is OK
- 10.47749/T/UNICAMP.2013.918998 is OK
- 10.1007/s101070100263 is OK
MISSING DOIs
- None
INVALID DOIs
- None
@whedon generate pdf from branch joss-paper-branch
Attempting PDF compilation from custom branch joss-paper-branch. Reticulating splines etc...
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
This is a very nice submission that was trivial to review. The paper and software are well written and a great fit for JOSS.
The only suggestion I had was to fix the plot (https://github.com/JuliaSmoothOptimizers/DCISolver.jl/issues/70), but other than that I'm happy for this to be accepted.
@whedon generate pdf from branch joss-paper-branch
Attempting PDF compilation from custom branch joss-paper-branch. Reticulating splines etc...
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
@tmigot - is it possible to enhance the documentation with parameters (function arguments), their types, their descriptions, return values, and examples as shown in here?
as shown in here?
This link might be better: https://docs.julialang.org/en/v1/manual/documentation/
@jbytecode @odow Thanks for the suggestion. I opened a PR that improve the documentation of the main functions. Would that answer your question?
looks better. thank you for adding documentation. but I think there is still some room to improve the documentation. you can add
to each function. if you need help please have a look at the popular repos, if you still need help I can suggest some.
thanks.
Ok, I updated only the external functions in the PR 74. I will also update the internal functions in the same way.
here is an example:
https://github.com/jbytecode/LinRegOutliers/blob/master/src/hs93.jl
one more thing:
Why are there some functions in test case, e.g., here
we can use nested test cases like
@testset "...." begin
@testset "...." begin
....
end
end
so you don't need to wrap more than one testcase
in a function and then call this function manually in test environments. This is unusual. Please tell me if there is a specific reason to do that.
Thanks.
Thanks for the suggestion, there was indeed no specific reason for that. I opened a PR to fix this.
thank you for the improvements, the software is in a better form by now on.
I am now passing my turn and giving the rubber back to the reviewers @jbcaillau
:wave: @jbcaillau, please update us on how your review is going (this is an automated reminder).
@jbcaillau could you please update your status and inform us on how is your review going?
Thank you in advance.
@jbcaillau we would like to hear about your review, sorry for pinging if I bothered you.
thank you in advance.
@jbytecode thanks for the reminder, taking care of this today
@jbcaillau we would like to hear about your review, sorry for pinging if I bothered you.
thank you in advance.
@tmigot @jbytecode Hi, I’ve read the paper and tried the code:
it seems a community guidelines part is missing, right?
@tmigot please consider the suggestions and apply the needed stuff if you are agreed with them. Please ping us when you have done. Thank you in advance
Dear @tmigot
Since there is an item in the reviewers' item list that states
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
please add some related sections in your README.md of repo.
Thank you in advance.
Hi @jbcaillau @jbytecode I tried to add some new features following your recommendations:
The link to the new tutorial: https://juliasmoothoptimizers.github.io/DCISolver.jl/dev/example/
@jbcaillau to re-run the benchmark from the paper, you can:
joss-paper-branch
; then move to the paper
folder.benchmark.jl
which will generate a .jld2 file with the result, and finally run figures.jl
to generate the figures.@tmigot thanks for the update!
binder
, I don’t see any toml
file indicating the dependencies: how is this supposed to work?binder
link launches the notebook but no kernel is foundOk, I finally figured it out, the link should be working with Julia now. It is slightly tricky to get the environment, so I added a link toward the Project.toml and the Manifest.toml at the beginning of the carnet.
@tmigot well done! see also below to speed up the process (double repo) https://discourse.jupyter.org/t/tip-speed-up-binder-launches-by-pulling-github-content-in-a-binder-link-with-nbgitpuller/922
Thanks for the link, I didn't know nbgitpuller
.
It is true that we have to rebuild the binder environment every time the repo is modified, but this guarantees that the notebook is up-to-date with the last version of the package.
@jbcaillau just because you wrote
I’ve seen that @jbytecode pointed some missing info in the code doc; nothing else from my side
in your previous entries, may I ask if you forgot to check the corresponding reviewing item? (and possibly others?)
Thank you.
@jbytecode @tmigot Hi, thank you for your patience:
All in all, this is a very nice and welcome piece of work so I do not want to delay its publication any longer on JOSS. Well done, @tmigot et al!
@jbcaillau what kind of trouble you had, could you please clarify?
Submissions on JOSS require software testing in any means, btw, in our case, this is a Julia package, so we expect
julia>] test DCISolver
to pass all of the tests in Julia REPL. Please check.
And, of course, the test directory should be capable of testing the whole of the functionality that the package serves.
In my Julia 1.7 setup, the test procedure produces the following output and it is a little bit verbose, btw, seems okay:
Testing Running tests...
[ Info: libhsl_ma57 not defined.
Test Summary: | Pass Total
Positive definite systems | 3 3
Test Summary: | Pass Total
Indefinite systems | 4 4
Test Summary: | Pass Total
Failed factorization | 2 2
Test Summary: | Pass Total
SQD system | 4 4
Test Summary: | Pass Total
LDLFactorizations with regularization | 3 3
[ Info: stage iter #f+#c f(x) ℓ ‖∇L‖ ‖c(x)‖ ρmax ρ status ‖d‖ Δ
[ Info: init 0 2 0.0e+00 0.0e+00 1.4e+00 0.0e+00 7.1e+01 NaN - - -
[ Info: Tr 0 4 -1.4e+00 -1.4e+00 - 0.0e+00 - - success 1.0e+00 2.0e+00
[ Info: T 0 4 -1.4e+00 -1.4e+00 1.4e+00 0.0e+00 7.1e+01 4.1e+01 success - -
[ Info: Tr 0 6 -3.0e+01 -3.0e+01 - 0.0e+00 - - success 2.0e+01 4.0e+01
[ Info: T 1 6 -3.0e+01 -3.0e+01 1.4e+00 0.0e+00 7.1e+01 4.1e+01 success - -
[ Info: Tr 0 8 -6.0e+02 -6.0e+02 - 0.0e+00 - - success 4.0e+02 8.0e+02
[ Info: T 2 8 -6.0e+02 -6.0e+02 1.4e+00 0.0e+00 7.1e+01 4.1e+01 success - -
[ Info: Tr 0 10 -1.2e+04 -1.2e+04 - 0.0e+00 - - success 8.0e+03 1.6e+04
[ Info: T 3 10 -1.2e+04 -1.2e+04 1.4e+00 0.0e+00 7.1e+01 4.1e+01 success - -
[ Info: Tr 0 12 -2.4e+05 -2.4e+05 - 0.0e+00 - - success 1.6e+05 3.2e+05
[ Info: T 4 12 -2.4e+05 -2.4e+05 1.4e+00 0.0e+00 7.1e+01 4.1e+01 success - -
Test Summary: | Pass Total
Unbounded tests | 1 1
Test Summary: | Pass Total
Unconstrained tests | 20 20
Test Summary: | Pass Total
Small equality constrained problems | 28 28
Test Summary: | Pass Total
HS7 | 4 4
stats.solution = [4.601595018913944, 1.9558435484179806]
Test Summary: | Pass Total
HS8 | 3 3
Test Summary: | Pass Total
HS9 | 3 3
stats.solution = [0.9976722156536222, 0.9976721869489701, 1.0023156818519954]
Test Summary: | Pass Total
HS26 | 3 3
stats.solution = [-1.0, 1.0000000000000075, -3.988238127092249e-9]
Test Summary: | Pass Total
HS27 | 3 3
[ Info: F 0 1 - - - 1.0e+00 - - too_small 0.0e+00 1.0e+00
[ Info: F-safe 0 2 - - - 1.0e+00 - - agressive_fail 0.0e+00 1.0e+00
[ Info: F 0 4 - - - 1.0e+00 - - reduce_Δ 1.0e+00 2.5e-01
[ Info: F 1 5 - - - 1.0e+00 - - reduce_Δ 2.5e-01 6.2e-02
[ Info: F 2 6 - - - 1.0e+00 - - reduce_Δ 6.2e-02 1.6e-02
[ Info: F 3 7 - - - 1.0e+00 - - reduce_Δ 1.6e-02 3.9e-03
[ Info: F 4 8 - - - 1.0e+00 - - success 3.9e-03 3.9e-03
[ Info: F 5 9 - - - 1.0e+00 - - success 3.9e-03 3.9e-03
[ Info: F 6 10 - - - 1.0e+00 - - success 3.9e-03 3.9e-03
[ Info: F-safe 6 11 - - - 1.0e+00 - - success 1.1e-02 3.9e-03
[ Info: F 7 12 - - - 1.0e+00 - - success 3.9e-03 7.8e-03
[ Info: F-safe 7 13 - - - 1.0e+00 - - success 3.9e-03 7.8e-03
[ Info: F 8 14 - - - 1.0e+00 - - success 7.8e-03 1.6e-02
[ Info: F-safe 8 15 - - - 1.0e+00 - - success 7.8e-03 1.6e-02
[ Info: F 9 16 - - - 1.0e+00 - - success 1.6e-02 3.1e-02
[ Info: F-safe 9 17 - - - 1.0e+00 - - success 1.6e-02 3.1e-02
[ Info: F 10 18 - - - 1.0e+00 - - success 3.1e-02 6.2e-02
[ Info: F-safe 10 19 - - - 1.0e+00 - - success 3.1e-02 6.2e-02
[ Info: F 11 20 - - - 1.0e+00 - - success 6.2e-02 1.2e-01
[ Info: F-safe 11 21 - - - 1.0e+00 - - success 6.3e-02 1.2e-01
[ Info: F 12 22 - - - 9.8e-01 - - success 1.2e-01 2.5e-01
[ Info: F-safe 12 23 - - - 9.8e-01 - - success 1.3e-01 2.5e-01
[ Info: F 13 24 - - - 9.4e-01 - - success 2.5e-01 5.0e-01
[ Info: F-safe 13 25 - - - 9.4e-01 - - success 2.9e-01 5.0e-01
[ Info: F 14 26 - - - 7.1e-01 - - success 5.0e-01 1.0e+00
[ Info: F 15 27 - - - 7.1e-01 - - reduce_Δ 6.5e-01 2.5e-01
[ Info: F 16 28 - - - 3.2e-02 - - success 1.8e-01 2.5e-01
Test Summary: | Pass Total
Example 1 | 2 2
[ Info: F 0 1 - - - 1.0e+00 - - too_small 0.0e+00 1.0e+00
[ Info: F-safe 0 2 - - - 1.0e+00 - - agressive_fail 0.0e+00 1.0e+00
[ Info: F 0 4 - - - 6.0e-03 - - success 1.0e+00 2.0e+00
Test Summary: | Pass Total
Example 2, Mission: Impossible | 2 2
Testing DCISolver tests passed
@tmigot In the README.md file the title "Bug reports and discussions" is okay, however, please add some information here on how to contribute to software by means of adding new functionality, improving the methods, etc. because the current information is only about reporting bugs. Thank you in advance.
@tmigot In the README.md file the title "Bug reports and discussions" is okay, however, please add some information here on how to contribute to software by means of adding new functionality, improving the methods, etc. because the current information is only about reporting bugs. Thank you in advance.
Thank you for the suggestion. Could you be more specific?
We wrote Focused suggestions and requests can also be opened as issues. Before opening a pull request, start an issue or a discussion on the topic, please.
in this spirit. The idea is to open an issue to discuss new functionalities.
If you mean to specify the future projects for the package, we also use the Github Projects.
please follow the sample repo here: https://github.com/jbytecode/JMcDM
the design pattern of implementing a new functionality is described.
if this is not current for the submission and implementing a new feature is not the case, you can leave it as is.
@jbytecode Hi, test passed too on 1.7, see below
Testing Running tests...
[ Info: libhsl_ma57 not defined.
Test Summary: | Pass Total
Positive definite systems | 3 3
Test Summary: | Pass Total
Indefinite systems | 4 4
Test Summary: | Pass Total
Failed factorization | 2 2
Test Summary: | Pass Total
SQD system | 4 4
Test Summary: | Pass Total
LDLFactorizations with regularization | 3 3
[ Info: stage iter #f+#c f(x) ℓ ‖∇L‖ ‖c(x)‖ ρmax ρ status ‖d‖ Δ
[ Info: init 0 2 0.0e+00 0.0e+00 1.4e+00 0.0e+00 7.1e+01 NaN - - -
[ Info: Tr 0 4 -1.4e+00 -1.4e+00 - 0.0e+00 - - success 1.0e+00 2.0e+00
[ Info: T 0 4 -1.4e+00 -1.4e+00 1.4e+00 0.0e+00 7.1e+01 4.1e+01 success - -
[ Info: Tr 0 6 -3.0e+01 -3.0e+01 - 0.0e+00 - - success 2.0e+01 4.0e+01
[ Info: T 1 6 -3.0e+01 -3.0e+01 1.4e+00 0.0e+00 7.1e+01 4.1e+01 success - -
[ Info: Tr 0 8 -6.0e+02 -6.0e+02 - 0.0e+00 - - success 4.0e+02 8.0e+02
[ Info: T 2 8 -6.0e+02 -6.0e+02 1.4e+00 0.0e+00 7.1e+01 4.1e+01 success - -
[ Info: Tr 0 10 -1.2e+04 -1.2e+04 - 0.0e+00 - - success 8.0e+03 1.6e+04
[ Info: T 3 10 -1.2e+04 -1.2e+04 1.4e+00 0.0e+00 7.1e+01 4.1e+01 success - -
[ Info: Tr 0 12 -2.4e+05 -2.4e+05 - 0.0e+00 - - success 1.6e+05 3.2e+05
[ Info: T 4 12 -2.4e+05 -2.4e+05 1.4e+00 0.0e+00 7.1e+01 4.1e+01 success - -
Test Summary: | Pass Total
Unbounded tests | 1 1
Test Summary: | Pass Total
Unconstrained tests | 20 20
Test Summary: | Pass Total
Small equality constrained problems | 28 28
Test Summary: | Pass Total
HS7 | 4 4
stats.solution = [4.601595018913945, 1.9558435484179804]
Test Summary: | Pass Total
HS8 | 3 3
Test Summary: | Pass Total
HS9 | 3 3
stats.solution = [0.9976722156536222, 0.9976721869489701, 1.0023156818519954]
Test Summary: | Pass Total
HS26 | 3 3
stats.solution = [-1.0, 1.0000000000000075, -3.988238127092249e-9]
Test Summary: | Pass Total
HS27 | 3 3
[ Info: F 0 1 - - - 1.0e+00 - - too_small 0.0e+00 1.0e+00
[ Info: F-safe 0 2 - - - 1.0e+00 - - agressive_fail 0.0e+00 1.0e+00
[ Info: F 0 4 - - - 1.0e+00 - - reduce_Δ 1.0e+00 2.5e-01
[ Info: F 1 5 - - - 1.0e+00 - - reduce_Δ 2.5e-01 6.2e-02
[ Info: F 2 6 - - - 1.0e+00 - - reduce_Δ 6.2e-02 1.6e-02
[ Info: F 3 7 - - - 1.0e+00 - - reduce_Δ 1.6e-02 3.9e-03
[ Info: F 4 8 - - - 1.0e+00 - - success 3.9e-03 3.9e-03
[ Info: F 5 9 - - - 1.0e+00 - - success 3.9e-03 3.9e-03
[ Info: F 6 10 - - - 1.0e+00 - - success 3.9e-03 3.9e-03
[ Info: F-safe 6 11 - - - 1.0e+00 - - success 1.1e-02 3.9e-03
[ Info: F 7 12 - - - 1.0e+00 - - success 3.9e-03 7.8e-03
[ Info: F-safe 7 13 - - - 1.0e+00 - - success 3.9e-03 7.8e-03
[ Info: F 8 14 - - - 1.0e+00 - - success 7.8e-03 1.6e-02
[ Info: F-safe 8 15 - - - 1.0e+00 - - success 7.8e-03 1.6e-02
[ Info: F 9 16 - - - 1.0e+00 - - success 1.6e-02 3.1e-02
[ Info: F-safe 9 17 - - - 1.0e+00 - - success 1.6e-02 3.1e-02
[ Info: F 10 18 - - - 1.0e+00 - - success 3.1e-02 6.2e-02
[ Info: F-safe 10 19 - - - 1.0e+00 - - success 3.1e-02 6.2e-02
[ Info: F 11 20 - - - 1.0e+00 - - success 6.2e-02 1.2e-01
[ Info: F-safe 11 21 - - - 1.0e+00 - - success 6.3e-02 1.2e-01
[ Info: F 12 22 - - - 9.8e-01 - - success 1.2e-01 2.5e-01
[ Info: F-safe 12 23 - - - 9.8e-01 - - success 1.3e-01 2.5e-01
[ Info: F 13 24 - - - 9.4e-01 - - success 2.5e-01 5.0e-01
[ Info: F-safe 13 25 - - - 9.4e-01 - - success 2.9e-01 5.0e-01
[ Info: F 14 26 - - - 7.1e-01 - - success 5.0e-01 1.0e+00
[ Info: F 15 27 - - - 7.1e-01 - - reduce_Δ 6.5e-01 2.5e-01
[ Info: F 16 28 - - - 3.2e-02 - - success 1.8e-01 2.5e-01
Test Summary: | Pass Total
Example 1 | 2 2
[ Info: F 0 1 - - - 1.0e+00 - - too_small 0.0e+00 1.0e+00
[ Info: F-safe 0 2 - - - 1.0e+00 - - agressive_fail 0.0e+00 1.0e+00
[ Info: F 0 4 - - - 6.0e-03 - - success 1.0e+00 2.0e+00
Test Summary: | Pass Total
Example 2, Mission: Impossible | 2 2
Testing DCISolver tests passed
@tmigot Still having an issue on 1.7 to run the benchmark; problem to precompile Ipopt, see
ERROR: LoadError: Failed to precompile Ipopt [b6b21f68-93f8-5de0-b562-5493be1d77c9] to /home/caillau/.julia/compiled/v1.7/Ipopt/jl_NNHi1z.
Hi @jbcaillau ! Can you give me more information on what you are trying to run exactly? Which versions of the packages? This seems to be unrelated to DCISolver, but with Ipopt.
@jbcaillau could you please update your status? thanks.
@tmigot @jbytecode Hi guys, sorry for being unclear on this:
Regards
Hi @jbcaillau ! Can you give me more information on what you are trying to run exactly? Which versions of the packages? This seems to be unrelated to DCISolver, but with Ipopt.
@jbcaillau thank you for the response. could you please check the corresponding review item in your reviewing list?
After finalizing the task, I will start my editorial stuff.
Thank you!
@whedon check references
Submitting author: @tmigot (Tangi Migot) Repository: https://github.com/JuliaSmoothOptimizers/DCISolver.jl Version: v0.2.6 Editor: @jbytecode Reviewer: @odow, @jbcaillau Archive: 10.5281/zenodo.6040222
:warning: JOSS reduced service mode :warning:
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
@odow & @jbcaillau, 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 @jbytecode 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 @odow
✨ 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 @jbcaillau
✨ 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