openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
721 stars 38 forks source link

[REVIEW]: SARAS: A general-purpose PDE solver for fluid dynamics #2095

Closed whedon closed 3 years ago

whedon commented 4 years ago

Submitting author: @roshansamuel (Roshan Samuel) Repository: https://github.com/roshansamuel/saras Version: v1.0 Editor: @kyleniemeyer Reviewers: @dlagrava, @olgadoronina, @nickwimer Archive: 10.5281/zenodo.5205320

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/7c3db3496426cd4abafef053c65ca825"><img src="https://joss.theoj.org/papers/7c3db3496426cd4abafef053c65ca825/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/7c3db3496426cd4abafef053c65ca825/status.svg)](https://joss.theoj.org/papers/7c3db3496426cd4abafef053c65ca825)

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

@harpolea & @dlagrava, 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 @kyleniemeyer know.

Please try and complete your review in the next two weeks

Review checklist for @dlagrava

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @olgadoronina

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @nickwimer

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @harpolea, @dlagrava it looks like you're currently assigned to review this paper :tada:.

: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:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

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 4 years ago
Reference check summary:

OK DOIs

- 10.1017/S0305004100023197 is OK
- 10.1063/1.1761178 is OK
- 10.1016/j.jpdc.2017.10.014 is OK
- 10.1098/rspa.1937.0036 is OK
- 10.1063/1.1761178 is OK
- 10.1137/1.9780898719505 is OK
- 10.1016/0021-9991(82)90058-4 is OK
- 10.1016/0017-9310(72)90054-3 is OK
- 10.1098/rspa.1991.0076 is OK
- 10.1007/BF01448839 is OK
- 10.1007/978-3-642-84108-8 is OK
- 10.1007/978-3-642-56026-2 is OK
- 10.1007/3-540-49372-7_24 is OK
- 10.1088/1742-6596/899/2/022017 is OK
- 10.1007/s12043-013-0594-4 is OK
- 10.1017/9781316810019 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

kyleniemeyer commented 4 years ago

@roshansamuel @harpolea @dlagrava 👋 the review takes place in this issue

dlagrava commented 4 years ago

@kyleniemeyer sounds good!

dlagrava commented 4 years ago

@roshansamuel I have been trying to compile the code on Ubuntu 19.04. It seems that the yaml-cpp-dev that I get is newer than the version you used. I get the following: error: ‘class YAML::Parser’ has no member named ‘GetNextDocument’; did you mean ‘HandleNextDocument’

It looks like you need yaml-cpp 0.3 and I get the 0.5 version from the package manager. Could you correct to ask for that particular version? ie. do not say to use the debian one for instance, but use the one you propose to download.

Cheers!

roshansamuel commented 4 years ago

@dlagrava Thank you for pointing this out. We had tested the code all along with yaml-cpp 0.3 and were not aware of this incompatibility with 0.5. We are working on updating the code (and the documentation), and we will get back to you soon.

roshansamuel commented 4 years ago

Hi @dlagrava we now indicate the yaml-cpp compatibility issues in the README. We are testing the new yaml API on a development branch and update the solver once it is ready.

dlagrava commented 4 years ago

@roshansamuel thanks for that! I will continue with my review then. Cheers.

harpolea commented 4 years ago

@roshansamuel I've had a few issues installing the required libraries as well. I also had issues with the yaml version, and it took me a while to get the blitz library installed as it seems to require python 2 (fortunately I was able to modify a single print statement to get it to work with python 3). I also had to modify the Cmake file to tell it which HDF5 header file to include (it may have got confused because I have multiple versions installed on this machine? I'm not familiar with Cmake so not sure).

Now I've fixed those issues it seems to have compiled successfully, so shall start testing

harpolea commented 4 years ago

@roshansamuel I've been trying to run the test problems and have been getting the following error. I've checked my yaml version and it's 0.3.0, so I'm not entirely sure what's gone wrong here?

[ 87%] Linking CXX executable ../../../../saras
[100%] Built target saras
terminate called after throwing an instance of 'YAML::InvalidScalar'
  what():  yaml-cpp: error at line 94, column 31: invalid scalar
[xrb:01852] *** Process received signal ***
[xrb:01852] Signal: Aborted (6)
[xrb:01852] Signal code:  (-6)
[xrb:01852] [ 0] /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20)[0x7fc127860f20]
[xrb:01852] [ 1] /lib/x86_64-linux-gnu/libc.so.6(gsignal+0xc7)[0x7fc127860e97]
[xrb:01852] [ 2] /lib/x86_64-linux-gnu/libc.so.6(abort+0x141)[0x7fc127862801]
[xrb:01852] [ 3] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x9987e)[0x7fc12849987e]
[xrb:01852] [ 4] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xa5486)[0x7fc1284a5486]
[xrb:01852] [ 5] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xa54f1)[0x7fc1284a54f1]
[xrb:01852] [ 6] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xa5745)[0x7fc1284a5745]
[xrb:01852] [ 7] ./saras(_ZN4YAMLrsIiEENS_9enable_ifINS_21is_scalar_convertibleIT_EEvE4typeERKNS_4NodeERS3_+0x88)[0x5585b2213978]
[xrb:01852] [ 8] ./saras(_ZN6parser9parseYAMLEv+0x153b)[0x5585b220dbfb]
[xrb:01852] [ 9] ./saras(_ZN6parserC1Ev+0xc6)[0x5585b2210206]
[xrb:01852] [10] ./saras(main+0x42)[0x5585b217a572]
[xrb:01852] [11] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)[0x7fc127843b97]
[xrb:01852] [12] ./saras(_start+0x2a)[0x5585b217c9ea]
[xrb:01852] *** End of error message ***
terminate called after throwing an instance of 'YAML::InvalidScalar'
  what():  yaml-cpp: error at line 94, column 31: invalid scalar
[xrb:01853] *** Process received signal ***
[xrb:01853] Signal: Aborted (6)
[xrb:01853] Signal code:  (-6)
[xrb:01853] [ 0] /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20)[0x7f5b37e59f20]
[xrb:01853] [ 1] /lib/x86_64-linux-gnu/libc.so.6(gsignal+0xc7)[0x7f5b37e59e97]
[xrb:01853] [ 2] /lib/x86_64-linux-gnu/libc.so.6(abort+0x141)[0x7f5b37e5b801]
[xrb:01853] [ 3] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x9987e)[0x7f5b38a9287e]
[xrb:01853] [ 4] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xa5486)[0x7f5b38a9e486]
[xrb:01853] [ 5] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xa54f1)[0x7f5b38a9e4f1]
[xrb:01853] [ 6] [xrb:01851] *** Process received signal ***
terminate called after throwing an instance of 'YAML::InvalidScalar'
  what():  yaml-cpp: error at line 94, column 31: invalid scalar
/usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xa5745)[0x7f5b38a9e745]
[xrb:01853] [ 7] [xrb:01851] Signal: Aborted (6)
[xrb:01851] Signal code:  (-6)
./saras(_ZN4YAMLrsIiEENS_9enable_ifINS_21is_scalar_convertibleIT_EEvE4typeERKNS_4NodeERS3_+0x88)[0x5599bdf62978]
[xrb:01853] [ 8] [xrb:01851] [ 0] ./saras(_ZN6parser9parseYAMLEv+0x153b)[0x5599bdf5cbfb]
[xrb:01853] [ 9] /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20)[0x7f43d8c01f20]
[xrb:01851] [ 1] ./saras(_ZN6parserC1Ev+0xc6)[0x5599bdf5f206]
[xrb:01853] [10] /lib/x86_64-linux-gnu/libc.so.6(gsignal+0xc7)[0x./saras(main+0x42)[0x5599bdec9572]
[xrb:01853] [11] 7f43d8c01e97]
[xrb:01851] [ 2] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)[0x7f5b37e3cb97]
[xrb:01853] [12] /lib/x86_64-linux-gnu/libc.so.6(abort+0x141)[0x7f43d8c03801]
[xrb:01851] [ 3] ./saras(_start+0x2a)[0x5599bdecb9ea]
[xrb:01853] *** End of error message ***
/usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x9987e)[0x7f43d983a87e]
[xrb:01851] [ 4] terminate called after throwing an instance of 'YAML::InvalidScalar'
  what():  yaml-cpp: error at line 94, column 31: invalid scalar
[xrb:01854] *** Process received signal ***
[xrb:01854] Signal: Aborted (6)
[xrb:01854] Signal code:  (-6)
/usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xa5486)[0x7f43d9846486]
[xrb:01851] [ 5] [xrb:01854] [ 0] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xa54f1)[0x7f43d98464f1]
[xrb:01851] [ 6] /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20)[0x7f9e6bca4f20]
[xrb:01854] [ 1] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xa5745)[0x7f43d9846745]
[xrb:01851] [ 7] /lib/x86_64-linux-gnu/libc.so.6(gsignal+0xc7)[0x7f9e6bca4e97]
[xrb:01854] [ 2] ./saras(_ZN4YAMLrsIiEENS_9enable_ifINS_21is_scalar_convertibleIT_EEvE4typeERKNS_4NodeERS3_+0x88)[0x56524cec9978]
[xrb:01851] [ 8] /lib/x86_64-linux-gnu/libc.so.6(abort+0x141)[0x7f9e6bca6801]
[xrb:01854] [ 3] ./saras(_ZN6parser9parseYAMLEv+0x153b)[0x56524cec3bfb]
[xrb:01851] [ 9] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x9987e)[0x7f9e6c8dd87e]
[xrb:01854] ./saras(_ZN6parserC1Ev+0xc6)[0x56524cec6206]
[xrb:01851] [10] [ 4] ./saras(main+0x42)[0x56524ce30572]
[xrb:01851] [11] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xa5486)[0x7f9e6c8e9486]
[xrb:01854] [ 5] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)[0x7f43d8be4b97]
[xrb:01851] [12] ./saras(_start+0x2a)[0x56524ce329ea]
[xrb:01851] *** End of error message ***
/usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xa54f1)[0x7f9e6c8e94f1]
[xrb:01854] [ 6] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xa5745)[0x7f9e6c8e9745]
[xrb:01854] [ 7] ./saras(_ZN4YAMLrsIiEENS_9enable_ifINS_21is_scalar_convertibleIT_EEvE4typeERKNS_4NodeERS3_+0x88)[0x55fdf942e978]
[xrb:01854] [ 8] ./saras(_ZN6parser9parseYAMLEv+0x153b)[0x55fdf9428bfb]
[xrb:01854] [ 9] ./saras(_ZN6parserC1Ev+0xc6)[0x55fdf942b206]
[xrb:01854] [10] ./saras(main+0x42)[0x55fdf9395572]
[xrb:01854] [11] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)[0x7f9e6bc87b97]
[xrb:01854] [12] ./saras(_start+0x2a)[0x55fdf93979ea]
[xrb:01854] *** End of error message ***
--------------------------------------------------------------------------
mpirun noticed that process rank 0 with PID 0 on node xrb exited on signal 6 (Aborted).
--------------------------------------------------------------------------
validate_ldc.py:80: YAMLLoadWarning: calling yaml.load() without Loader=... is deprecated, as the default Loader is unsafe. Please read https://msg.pyyaml.org/load for full details.
  yamlData = yl.load(yamlFile)
Could not open file output/Soln_0030.0000.h5
dlagrava commented 4 years ago

@roshansamuel I encountered the same problems as @harpolea . It is a good idea to have a CMake, but you can probably improve on that, especially because people may want to have a local blitz install etc. I also ran into the same problem:

[100%]` Built target saras
terminate called after throwing an instance of 'YAML::InvalidScalar'
  what():  yaml-cpp: error at line 94, column 31: invalid scalarterminate called after throwing an instance of 'YAML::InvalidScalar'
  what():  yaml-cpp: error at line 94, column 31: invalid scalar
terminate called after throwing an instance of 'YAML::InvalidScalar'
  what():  yaml-cpp: error at line 94, column 31: invalid scalar
terminate called after throwing an instance of 'YAML::InvalidScalar'
  what():  yaml-cpp: error at line 94, column 31: invalid scalar

[reReBoot:17122] *** Process received signal ***
[reReBoot:17122] Signal: Aborted (6)
[reReBoot:17122] Signal code:  (-6)
[reReBoot:17123] *** Process received signal ***
[reReBoot:17123] Signal: Aborted (6)
[reReBoot:17123] Signal code:  (-6)
[reReBoot:17120] *** Process received signal ***
[reReBoot:17120] Signal: Aborted (6)
[reReBoot:17120] Signal code:  (-6)
[reReBoot:17120] [ 0] /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20)[0x7f9a116b0f20]
[reReBoot:17120] [ 1] [reReBoot:17121] *** Process received signal ***
[reReBoot:17121] Signal: Aborted (6)
[reReBoot:17121] Signal code:  (-6)
[reReBoot:17121] [ 0] [reReBoot:17122] [ 0] /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20)[0x7f862a86df20]
[reReBoot:17122] [reReBoot:17123] [ 0] /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20)[0x7f8840f55f20]
[reReBoot:17123] [ 1] /lib/x86_64-linux-gnu/libc.so.6(gsignal+0xc7)[0x7f8840f55e97]
[reReBoot:17123] [ 2] /lib/x86_64-linux-gnu/libc.so.6(gsignal+0xc7)[0x7f9a116b0e97]
[reReBoot:17120] [ 2] /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20)[0x7f77b534af20]
[reReBoot:17121] [ 1] /lib/x86_64-linux-gnu/libc.so.6(gsignal[ 1] /lib/x86_64-linux-gnu/libc.so.6(gsignal+0xc7)[0x7f862a86de97]
[reReBoot:17122] [ 2] +0xc7)[0x7f77b534ae97]
[reReBoot:17121] [ 2] /lib/x86_64-linux-gnu/libc.so.6(abort+0x141)[0x7f9a116b2801]
[reReBoot:17120] [ 3] /lib/x86_64-linux-gnu/libc.so.6(abort+0x141)[0x7f862a86f801]
[reReBoot:17122] [ 3] /lib/x86_64-linux-gnu/libc.so.6(abort+0x141)[0x7f8840f57801]
[reReBoot:17123] [ 3] /lib/x86_64-linux-gnu/libc.so.6(abort+0x141)[0x7f77b534c801]
[reReBoot:17121] [ 3] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x8c957)[0x7f9a122d4957]
[reReBoot:17120] [ 4] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x8c957)[0x7f862b491957]
[reReBoot:17122] [ 4] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x8c957)[0x7f8841b79957]
[reReBoot:17123] [ 4] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x8c957)[0x7f77b5f6e957]
[reReBoot:17121] [ 4] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x92ab6)[0x7f8841b7fab6]
[reReBoot:17123] [ 5] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x92ab6)[0x7f9a122daab6]
[reReBoot:17120] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x92ab6)[0x7f862b497ab6]
[reReBoot:17122] [ 5] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x92af1)[0x7f8841b7faf1]
[reReBoot:17123] [ 6] [ 5] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x92ab6)[0x7f77b5f74ab6]
[reReBoot:17121] [ 5] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x92af1)[0x7f862b497af1]
[reReBoot:17122] [ 6] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x92af1)[0x7f9a122daaf1]
[reReBoot:17120] [ 6] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x92af1)[0x7f77b5f74af1]
[reReBoot:17121] [ 6] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x92d24)[0x7f8841b7fd24]
[reReBoot:17123] [ 7] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x92d24)[0x7f9a122dad24]
[reReBoot:17120] [ 7] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x92d24)[0x7f862b497d24]
[reReBoot:17122] [ 7] ./saras(_ZN4YAMLrsIiEENS_9enable_ifINS_21is_scalar_convertibleIT_EEvE4typeERKNS_4NodeERS3_+0x88)[0x561b327c3498]
[reReBoot:17123] [ 8] ./saras(_ZN4YAMLrsIiEENS_9enable_ifINS_21is_scalar_convertibleIT_EEvE4typeERKNS_4NodeERS3_+0x88)[0x55af1a046498]
[reReBoot:17120] [ 8] ./saras(_ZN6parser9parseYAMLEv+0x1747)[0x561b327bda57]
[reReBoot:17123] [ 9] ./saras(_ZN4YAMLrsIiEENS_9enable_ifINS_21is_scalar_convertibleIT_EEvE4typeERKNS_4NodeERS3_+0x88)[0x560d6873a498]
[reReBoot:17122] [ 8] /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x92d24)[0x7f77b5f74d24]
./saras(_ZN6parser9parseYAMLEv+0x1747)[0x55af1a040a57]
[reReBoot:17120] [ 9] ./saras(_ZN6parserC2Ev+0xc6)[0x55af1a042d26]
[reReBoot:17120] [10] [reReBoot:17121] [ 7] ./saras(_ZN4YAMLrsIiEENS_9enable_ifINS_21is_scalar_convertibleIT_EEvE4typeERKNS_4NodeERS3_+0x88)[0x5564c18f6498]
[reReBoot:17121] [ 8] ./saras(_ZN6parserC2Ev+0xc6)[0x561b327bfd26]
[reReBoot:17123] [10] ./saras(main+0x42)[0x561b3272bb62]
[reReBoot:17123] [11] ./saras(_ZN6parser9parseYAMLEv+0x1747)[0x560d68734a57]
[reReBoot:17122] [ 9] ./saras(_ZN6parserC2Ev+0xc6./saras(main+0x42)[0x55af19faeb62]
[reReBoot:17120] [11] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)[0x7f9a11693b97]
[reReBoot:17120] [12] )[0x560d68736d26]
[reReBoot:17122] [10] ./saras(main+0x42)[0x560d686a2b62]
./saras(_ZN6parser9parseYAMLEv+0x1747)[0x5564c18f0a57]
[reReBoot:17121] [ 9] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)[0x7f8840f38b97]
[reReBoot:17123] [12] ./saras(_start+0x2a)[0x55af19faf6ea]
[reReBoot:17120] *** End of error message ***
./saras(_start+0x2a)[0x561b3272c6ea]
[reReBoot:17123] *** End of error message ***
[reReBoot:17122] [11] ./saras(_ZN6parserC2Ev+0xc6)[0x5564c18f2d26]
[reReBoot:17121] [10] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)[0x7f862a850b97]
[reReBoot:17122] [12] ./saras(_start+0x2a)./saras(main+0x42)[0x5564c185eb62]
[reReBoot:17121] [11] [0x560d686a36ea]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)[0x7f77b532db97]
[reReBoot:17121] [12] [reReBoot:17122] *** End of error message ***
./saras(_start+0x2a)[0x5564c185f6ea]
[reReBoot:17121] *** End of error message ***
roshansamuel commented 4 years ago

Hi @harpolea and @dlagrava, thank you very much for bringing the Blitz installation issues to our notice. The latest version of Blitz available on GitHub at blitzpp/blitz has resolved the Python3 related issues. We are updating the installer provided by the link in documentation with this version of Blitz.

The YAML related error thrown when running the test module happened because I had failed to update the parameters YAML file of the LDC test input. I have corrected this and pushed an update to the repository. Many apologies for this lapse.

The error should be resolved now after a git pull.

dlagrava commented 4 years ago

@roshansamuel the fix allows me to execute the code and I can run the tests. As a general rule, and I swear I remember a requirement like this on my latest review, we should test a release that you have made sure compiles and works. I will be going through the document and the rest of the review now. Thanks!

arfon commented 4 years ago

Dear authors and reviewers

We wanted to notify you that in light of the current COVID-19 pandemic, JOSS has decided to suspend submission of new manuscripts and to handle existing manuscripts (such as this one) on a "best efforts basis". We understand that you may need to attend to more pressing issues than completing a review or updating a repository in response to a review. If this is the case, a quick note indicating that you need to put a "pause" on your involvement with a review would be appreciated but is not required.

Thanks in advance for your understanding.

Arfon Smith, Editor in Chief, on behalf of the JOSS editorial team.

arfon commented 4 years ago

:wave: @harpolea, @dlagrava, just a friendly check-in to see how things are going with your reviews?

dlagrava commented 4 years ago

@arfon I am really sorry, I haven't had almost any free time lately. We have two small children and with full lock-down here in New Zealand and I can tell you that it is hands-on to keep them busy. I will try to restart doing this in 2 weeks time, when schools may re-open and we may go back to some normality. Hope you are all well.

arfon commented 4 years ago

@dlagrava - please, no apology is necessary. I totally understand that everyone’s situation is different right now, hence I’ve been checking in on all active JOSS reviews to see how authors, editors, and reviewers are getting on.

Please take your time and focus on your personal situation.

arfon commented 4 years ago

:wave: @dlagrava - a friendly check in to see how things are going for you these days?

dlagrava commented 4 years ago

@arfon thanks for the message. Children were back to school this week, and I was able to catch-up with work stuff that was due. I will fast-track the review and finish it before June. Have a good day!

dlagrava commented 4 years ago

@roshansamuel Sorry for the delay, it is unprecedented times that we are currently living, hope that you are well.

I have mostly finished my review. I have 3 outstanding issues that I have opened on your repository about community guidelines, some missing documentation to write examples and intent of the software and comparison with existing software. I am happy to continue the discussion on each of the issues individually.

Thanks!

kyleniemeyer commented 4 years ago

Thanks @dlagrava!

I am linking the issues here for convenience: https://github.com/roshansamuel/saras/issues/1 https://github.com/roshansamuel/saras/issues/2 https://github.com/roshansamuel/saras/issues/3

roshansamuel commented 4 years ago

Thank you very much, @dlagrava and @kyleniemeyer! The current situation across the world is distressing indeed, and we hope the reviewers and editorial team are well. We are working on resolving the three issues opened on the repository. Many thanks for the helpful comments!

roshansamuel commented 4 years ago

Dear @kyleniemeyer, @dlagrava and @harpolea: In a recent discussion among the authors, we converged that two contributors, whose inputs were essential to the design of the solver, deserve to be included in the list of authors. Prof. Ravi Samtaney gave valuable ideas and greatly aided the development of multigrid pressure correction solver. At the same time, Prof. Fahad Anwer helped us in implementing the semi-implicit Crank-Nicholson scheme for time-integration.

We should have had included them in the beginning itself, but it was an oversight. As you all know, code development involves many personnel, and authorships tend to be confusing. During the lockdown, we had a careful review of our work and the present paper; that’s when we came to the above decision. Kindly note that the code remains unchanged (except for some more documentation, as advised by the reviewers).

We request you to allow us to include the two original contributors to SARAS as authors to the present paper. Kindly let us know if you need any clarifications.

kyleniemeyer commented 4 years ago

Hi @roshansamuel, that seems like a perfectly valid reason to me, as long as all the original authors agree to this. JOSS generally leaves authorship decisions up to the authors themselves, unless something seems off.

roshansamuel commented 4 years ago

Hi @kyleniemeyer and @dlagrava, out of the three issues opened on the repository, one was closed on Jun 4 following an update to the README. We have responded to Issue 2 by adding instructions on how to set up a new case in the README. To address Issue 3, we expanded the Summary section of the paper with a statement of intent, and a brief comparison with other similar software.

Please let us know if all the open issues on the repository can now be closed.

Thanks!

kyleniemeyer commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

kyleniemeyer commented 4 years ago

Hi @roshansamuel, I think your changes have addressed @dlagrava's concerns, thanks!

kyleniemeyer commented 4 years ago

Hi @harpolea it looks like some of your review checklist items are missing, can you complete your review here? Thanks!

labarba commented 4 years ago

👋 hi everybody!

It looks like this review was progressing, the authors addressed several issues, but activity stopped a few weeks ago. Can we have an update from everyone involved? @harpolea, @dlagrava : what is the status of your reviews? If you need more time, just let us know—times are tough!

dlagrava commented 4 years ago

Hello @labarba

Seems like I forgot to update the checklist, sorry, my bad.

I was the one that raised the issues and am happy with the results. From my side, even though the installation is still a little dodgy, I think that I agree with the rest of the checklist.

Cheers!

roshansamuel commented 4 years ago

Hi @dlagrava

Many thanks for reviewing SARAS and pointing out the issues that needed fixing. If something can be done to make its installation smooth, we will be very glad to implement that too. We thought that the included script to build SARAS would make its installation easy.

Thanks!

kyleniemeyer commented 4 years ago

Thanks @dlagrava!

kyleniemeyer commented 4 years ago

@harpolea did you ever finish your review?

roshansamuel commented 4 years ago

Hi @harpolea ,

Would it be possible to complete the review soon? Any comments to ease the installation and review of the software will be very welcome.

Thanks!

roshansamuel commented 4 years ago

Dear @kyleniemeyer ,

We are using data from simulations with SARAS in upcoming papers from our lab. It would be greatly helpful if the review of this software can be expedited. Once published, it would be easier for us to cite the solver in our papers.

Can something be done to bring the review back on track?

Thanks!

arfon commented 4 years ago

@kyleniemeyer - I think we're going to need to find an alternative reviewer here?

kyleniemeyer commented 4 years ago

Hi @adam-m-jcbs, are you available to review this submission for JOSS?

kyleniemeyer commented 3 years ago

@roshansamuel I'm sorry this is taking so long—I am actively seeking another reviewer.

kyleniemeyer commented 3 years ago

@whedon add @olgadoronina as reviewer

whedon commented 3 years ago

OK, @olgadoronina is now a reviewer

kyleniemeyer commented 3 years ago

@whedon add @nickwimer as reviewer

whedon commented 3 years ago

OK, @nickwimer is now a reviewer

kyleniemeyer commented 3 years ago

@whedon remove @harpolea as reviewer

whedon commented 3 years ago

OK, @harpolea is no longer a reviewer

kyleniemeyer commented 3 years ago

@olgadoronina @nickwimer I have added you as reviewers—thank you for agreeing! I hope you can submit your reviews within the next two weeks. You should be able to edit the checklists above, and our review criteria are described in more detail at https://joss.readthedocs.io/en/latest/review_criteria.html

Hi @roshansamuel, just to update you: I have found two new reviewers, and I hope we can have this wrapped up in the next few weeks—thanks for your patience.

roshansamuel commented 3 years ago

Hi @kyleniemeyer, thank you very much for assigning new reviewers. We look forward to the reviews.

kyleniemeyer commented 3 years ago

@whedon re-invite @olgadoronina as reviewer

whedon commented 3 years ago

OK, the reviewer has been re-invited.

@olgadoronina please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations