JuliaCon / proceedings-review

6 stars 1 forks source link

[REVIEW]: ExaPF.jl: A Power Flow Solver for GPUs #72

Closed whedon closed 8 months ago

whedon commented 3 years ago

Submitting author: !--author-handle-->@michel2323<!--end-author-handle-- (Michel Schanen) Repository: https://github.com/exanauts/JuliaCon2020 Branch with paper.md (empty if default branch): Version: Editor: !--editor-->@matbesancon<!--end-editor-- Reviewers: @mjamei19, @lcw Archive: Pending

Status

status

Status badge code:

HTML: <a href="https://proceedings.juliacon.org/papers/9e44c5835d87dab402f0eee34dcd36f5"><img src="https://proceedings.juliacon.org/papers/9e44c5835d87dab402f0eee34dcd36f5/status.svg"></a>
Markdown: [![status](https://proceedings.juliacon.org/papers/9e44c5835d87dab402f0eee34dcd36f5/status.svg)](https://proceedings.juliacon.org/papers/9e44c5835d87dab402f0eee34dcd36f5)

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

@mjamei19, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @vchuravy 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 @mjamei19

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Paper format

Content

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @mjamei19 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/JuliaCon/proceedings-review) 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:

  1. Set yourself as 'Not watching' https://github.com/JuliaCon/proceedings-review:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

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
whedon commented 3 years ago

Failed to discover a Statement of need section in paper

whedon commented 3 years ago
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.56 s (66.0 files/s, 13901.6 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Julia                           20            505            262           2438
TeX                              8            271            219           2430
Jupyter Notebook                 2              0            845            312
SVG                              1              1              1            274
Python                           1             13              3             59
Ruby                             1              8              4             45
YAML                             2              0              1             35
TOML                             1              4              0             32
Markdown                         1             12              0             24
-------------------------------------------------------------------------------
SUM:                            37            814           1335           5649
-------------------------------------------------------------------------------

Statistical information for the repository '5b6e4b91c10cfe42f1ab5f4c' was
gathered on 2021/03/01.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Adrian Maldonado                 2            78              3           58.70
Michel Schanen                   1            57              0           41.30

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Adrian Maldonado             75           96.2          0.3                4.00
Michel Schanen               57          100.0          0.0                7.02
whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

whedon commented 3 years ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1137/0913035 is OK
- 10.1109/TPDS.2018.2872064 is OK
- 10.1137/130908737 is OK

MISSING DOIs

- 10.1109/tsg.2016.2610863 may be a valid DOI for title: Parallel Dynamics Simulation Using a Krylov-Schwarz Linear Solution Scheme
- 10.1016/j.advengsoft.2019.02.002 may be a valid DOI for title: Rapid software prototyping for heterogeneous and distributed platforms
- 10.1145/3404397.3404470 may be a valid DOI for title: Vector Forward Mode Automatic Differentiation on SIMD/SIMT architectures 
- 10.1007/s10107-004-0559-y may be a valid DOI for title: On the implementation of an interior-point filter line-search algorithm for large-scale nonlinear programming
- 10.1109/tpwrs.2007.901301 may be a valid DOI for title: On Computational Issues of Market-Based Optimal Power Flow

INVALID DOIs

- None
vchuravy commented 3 years ago

@whedon add @lcw as reviewer

whedon commented 3 years ago

OK, @lcw is now a reviewer

whedon commented 3 years ago

:wave: @mjamei19, please update us on how your review is going (this is an automated reminder).

matbesancon commented 3 years ago

@whedon assign @matbesancon as editor

matbesancon commented 3 years ago

@whedon check references

matbesancon commented 3 years ago

@whedon list commands

whedon commented 3 years ago

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@whedon commands
matbesancon commented 3 years ago

@whedon commands

whedon commented 3 years ago

Here are some things you can ask me to do:

# List all of Whedon's capabilities
@whedon commands

# Assign a GitHub user as the sole reviewer of this submission
@whedon assign @username as reviewer

# Add a GitHub user to the reviewers of this submission
@whedon add @username as reviewer

# Re-invite a reviewer (if they can't update checklists)
@whedon re-invite @username as reviewer

# Remove a GitHub user from the reviewers of this submission
@whedon remove @username as reviewer

# List of editor GitHub usernames
@whedon list editors

# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers

# Change editorial assignment
@whedon assign @username as editor

# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive

# Set the software version at the top of the issue e.g.
@whedon set v1.0.1 as version

# Open the review issue
@whedon start review

EDITORIAL TASKS

# All commands can be run on a non-default branch, to do this pass a custom 
# branch name by following the command with `from branch custom-branch-name`.
# For example:

# Compile the paper
@whedon generate pdf

# Compile the paper from alternative branch
@whedon generate pdf from branch custom-branch-name

# Remind an author or reviewer to return to a review after a
# certain period of time (supported units days and weeks)
@whedon remind @reviewer in 2 weeks

# Ask Whedon to do a dry run of accepting the paper and depositing with Crossref
@whedon accept

# Ask Whedon to check the references for missing DOIs
@whedon check references

# Ask Whedon to check repository statistics for the submitted software
@whedon check repository

EiC TASKS

# Invite an editor to edit a submission (sending them an email)
@whedon invite @editor as editor

# Reject a paper
@whedon reject

# Withdraw a paper
@whedon withdraw

# Ask Whedon to actually accept the paper and deposit with Crossref
@whedon accept deposit=true
whedon commented 3 years ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1137/0913035 is OK
- 10.1109/TPDS.2018.2872064 is OK
- 10.1137/130908737 is OK

MISSING DOIs

- 10.1109/tsg.2016.2610863 may be a valid DOI for title: Parallel Dynamics Simulation Using a Krylov-Schwarz Linear Solution Scheme
- 10.1016/j.advengsoft.2019.02.002 may be a valid DOI for title: Rapid software prototyping for heterogeneous and distributed platforms
- 10.1145/3404397.3404470 may be a valid DOI for title: Vector Forward Mode Automatic Differentiation on SIMD/SIMT architectures 
- 10.1007/s10107-004-0559-y may be a valid DOI for title: On the implementation of an interior-point filter line-search algorithm for large-scale nonlinear programming
- 10.1109/tpwrs.2007.901301 may be a valid DOI for title: On Computational Issues of Market-Based Optimal Power Flow

INVALID DOIs

- None
matbesancon commented 3 years ago

hi @michel2323, it looks like you have some DOIs missing on the paper, could you see if the suggested missing DOIs apply to the references you cite

lcw commented 3 years ago

Review

In addition to reading the submitted paper, I checked out the code from https://github.com/exanauts/ExaPF.jl and tried to run it.

The submitted paper gives a short description of the need for optimal power flow solvers on the GPU, a detailed description of the technical components (autodiff and linear solvers), and finishes with a couple performance results. In all this is an ambitious project and I applaud the authors efforts. It is exciting to see more and more code in the julia ecosystem work for both the CPU and GPU.

Please find below a list of comments and concerns I have with the current presentation of the work.

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Paper format

Content

matbesancon commented 3 years ago

Thanks Lucas for the review. For the authors: please let us know when you have updated the paper to take the review into account and address the issues that were opened in the repo

frapac commented 3 years ago

Dear all,

First, we would like to thank the reviewer for his thorough and meaningful review. To our concern, all points raised by the reviewer are relevant.

We are currently addressing the most urgent tasks, as

Once these points are addressed, we are planning to proceed to a new release for ExaPF (v0.5.0).

Remains the last (but not least) concern raised by the reviewer, pointing to the performance of the GPU code versus the CPU code. We acknowledge that this claim is highlt legitimate: why bothering porting code on the GPU if overall the CPU code is faster? We give two answers to that concern, depending whether we look at things from the top or from the bottom up.

We hope that these two points allowed us to clarify some of the concerns of the reviewer.

Eventually, this raises the question of the current state of the article, whose results are lagging behind the most recent developments. The proceedings paper is based on the release as of JuliaCon 2020 and ran on Julia 1.4, and does not cover the latest development made in ExaPF. That’s the reason why we would like to propose two paths onward:

  1. either we address the concerns of the reviewer and leave the results unchanged, representing the status of submission of JuliaCon 2020 with Julia 1.4.
  2. or we withdraw the paper as is, and present a revamped version for JuliaCon 2021.

Michel Schanen & François Pacaud

matbesancon commented 3 years ago

Thanks François for the detailed answer. I would be okay with both paths onwards and will leave it to the authors to choose. One thing to note: the conference is continuously publishing the proceedings, so there is no need to choose specifically "publishing in JuliaCon 2020 or 2021". Given that, I don't think it is worth it to withdraw to resubmit, it would be the same thing as resubmitting. So specifically, I am okay with keeping the results are you ran them on the original submission if you prefer.

The second point of answer you mentioned above "Looking more closely at the performance..." offers great insight that is worth having in the paper IMO

michel2323 commented 3 years ago

@whedon generate pdf from branch paper

whedon commented 3 years ago
Attempting PDF compilation from custom branch paper. Reticulating splines etc...
whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

michel2323 commented 3 years ago

@whedon check references

whedon commented 3 years ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1137/0913035 is OK
- 10.1109/tsg.2016.2610863 is OK
- 10.1109/TPDS.2018.2872064 is OK
- 10.1016/j.advengsoft.2019.02.002 is OK
- 10.1145/3404397.3404470 is OK
- 10.1137/130908737 is OK
- 10.1007/s10107-004-0559-y is OK
- 10.1109/tpwrs.2007.901301 is OK

MISSING DOIs

- None

INVALID DOIs

- None
matbesancon commented 3 years ago

there seem to be an issue with the references

matbesancon commented 3 years ago

@whedon generate pdf from branch paper

whedon commented 3 years ago
Attempting PDF compilation from custom branch paper. Reticulating splines etc...
whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

matbesancon commented 3 years ago

@michel2323 you should use the same bibtex structure left as-is: comment out the bibliographystyle and bibliography insertion and uncomment the ones from below

matbesancon commented 3 years ago

bump

michel2323 commented 3 years ago

@whedon generate pdf from branch paper

whedon commented 3 years ago
Attempting PDF compilation from custom branch paper. Reticulating splines etc...
whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

matbesancon commented 3 years ago

Thanks the reference issue seems fixed now. @lcw can you confirm the new changes correspond to your previous comments and whether the manuscript is now good on your side?

lcw commented 3 years ago

The authors have addressed many of my issues and I understand that some of the ones I brought up are out of scope. The readme is greatly improved, however, I am unable to run the examples https://github.com/exanauts/ExaPF.jl#how-to-solve-the-power-flow-of-a-given-matpower-instance and https://github.com/exanauts/ExaPF.jl#how-to-solve-the-optimal-power-flow-in-the-reduced-space without modification.

That said, I am okay if the authors want to push forward and publish.

matbesancon commented 2 years ago

@michel2323 @frapac do you want to address the points above on the examples? Otherwise let us know if you did the changes you wanted

matbesancon commented 2 years ago

bump

frapac commented 2 years ago

Dear Mathieu, First, I am sorry for the late reply. We are planning to address the two last points raised by Lucas by the JuliaCon, to get everything working by then. Then, we will update the JuliaCon paper with our latest results, as discussed earlier in this discussion. Would that be ok for you?

matbesancon commented 2 years ago

no problem on my end, let us know when you have made the modifications

matbesancon commented 2 years ago

bump here, @frapac any progress?

matbesancon commented 2 years ago

bump @michel2323 @frapac

matbesancon commented 2 years ago

bump @michel2323 @frapac

matbesancon commented 2 years ago

bump @michel2323 @frapac

luraess commented 9 months ago

Dear author @michel2323, we are in the process of resuming the stalled JuliaCon reviews. If you are still interested in getting your work published, please start addressing the author-action-required items within the coming 3 weeks. If no action is taken within the coming 3 weeks, we will close the submission. Thank you for your understanding.

lucaferranti commented 2 months ago

@editorialbot reject

editorialbot commented 2 months ago

Paper rejected.