Submitting author: @pankajkmishra (Pankaj K Mishra)
Repository: https://github.com/pankajkmishra/NodeLab
Version: v1.0
Editor: @kyleniemeyer
Reviewer: @Kevin-Mattheus-Moerman, @vijaysm
Archive: 10.5281/zenodo.3361734
Status badge code:
HTML: <a href="http://joss.theoj.org/papers/74b8ffde0c0ba8342c88320814bbefcc"><img src="http://joss.theoj.org/papers/74b8ffde0c0ba8342c88320814bbefcc/status.svg"></a>
Markdown: [](http://joss.theoj.org/papers/74b8ffde0c0ba8342c88320814bbefcc)
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) 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.)
@Kevin-Mattheus-Moerman & @vijaysm, 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.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @kyleniemeyer know.
β¨ Please try and complete your review in the next two weeks β¨
paper.md
file include a list of authors with their affiliations?paper.md
file include a list of authors with their affiliations?Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @Kevin-Mattheus-Moerman, it looks like you're currently assigned as the reviewer for 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:
For a list of things I can do to help you, just type:
@whedon commands
Attempting PDF compilation. Reticulating splines etc...
π @pankajkmishra @Kevin-Mattheus-Moerman @vijaysm the actual review takes place in this issue. Please note the two separate review checklists above.
@whedon commands
Here are some things you can ask me to do:
# List Whedon's capabilities
@whedon commands
# List of editor GitHub usernames
@whedon list editors
# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers
EDITORIAL TASKS
# Compile the paper
@whedon generate pdf
# Ask Whedon to check the references for missing DOIs
@whedon check references
@kyleniemeyer: Is there anything required from my side at this point?
@pankajkmishra not at the momentβthe reviewers will report any issues requiring your attention here or in the software repository as appropriate.
I should be able to complete the full review by next Monday. It has been a busy couple of weeks. @pankajkmishra @kyleniemeyer
@@kyleniemeyer: Is it allowed to update the repository while it is under review?
I'll finalize the review by next Monday. Apologies for the delay.
@pankajkmishra Yes, but please let us know here when you update it and what changes were made; it sounds like the reviewers may not have started with the current version, so should be fine.
@kyleniemeyer Should I upload a list of my suggestions separately or just continue discussion with the author here as part of the comment thread ?
@pankajkmishra I also noticed that there is no official release for NodeLab yet. So it may be appropriate to update the versioning info first and then tag the commit as a checkpoint for the review. Thoughts @kyleniemeyer ?
@vijaysm feel free to continue the discussion here; you can submit a list of suggestions as a single comment and we can go from there.
Regarding the official release, I'm fine if @pankajkmishra waits until after making any changes as part of this review process, and then makes an official release associated with the version accepted here.
Thanks @vijaysm! I'll look forward to your suggestions. @kyleniemeyer - That would be convenient! Thanks.
@pankajkmishra Here are some comments on the paper:
[x] Add "be" in the following: "....(Persson & Strang, 2004), which can [be] computed based on a..."
[x] Remove plural in: "As a result~s~, NodeLab can take the following..."
[x] Rephrase "some discrete set of point cloud on the boundary, which need not to be uniformly
sampled." e.g. to: "some discrete ~set of~ point cloud on the boundary, which need not ~to~ be uniformly
sampled."
[x] Can you work to rephrase/clarify this awkward sounding sentence: "The boundary can be smoothed through curve-interpolation according to the fill-distance near the boundary, which provides the flexibility to create the domain by manually digitizing of the geometry from a hand-drawing, digital-drawing, or a downloaded image."
[x] Can you add a References heading?
@whedon check references
Attempting to check references...
```Reference check summary:
OK DOIs
MISSING DOIs
INVALID DOIs
I've created a pull request (https://github.com/pankajkmishra/NodeLab/pull/2) to enhance the demos/documentation. I recommend clarifying the required input arguments and perhaps to visualize them as I suggest.
I also recommend making use of the MATLAB documentation generation tools. I give an example in that demo. You can publish it to create HTML documentation which can be integrated in MATLAB.
Thanks @Kevin-Mattheus-Moerman! I'll get back to you after implementing your suggestions.
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
@pankajkmishra great. Can you also work on adding those DOI's? Let me know if you have questions.
@whedon check references
Attempting to check references...
@pankajkmishra - there were some missing commas in your bibtex: https://github.com/pankajkmishra/NodeLab/pull/3
Thanks @arfon !
@whedon check references
Attempting to check references...
```Reference check summary:
OK DOIs
MISSING DOIs
INVALID DOIs
```Reference check summary:
OK DOIs
MISSING DOIs
INVALID DOIs
Hi @Kevin-Mattheus-Moerman ! I have updated the paper according to the following comments. I have also merged your expanded visualization for the first test, which would (indeed) provide more insight to the user. However, I have not included this visualization to other tests - to keep the demos simple and easy to integrate in external (meshfree) PDE solvers. I have added the description of Input/Output variables in the 'readme' file.
- [x] Add "be" in the following: "....(Persson & Strang, 2004), which can [be] computed based on a..." - [x] Remove plural in: "As a results, NodeLab can take the following..." - [x] Rephrase "some discrete set of point cloud on the boundary, which need not to be uniformly sampled." e.g. to: "some discrete set of point cloud on the boundary, which need not to be uniformly sampled." [x] Can you work to rephrase/clarify this awkward sounding sentence: "The boundary can be smoothed through curve-interpolation according to the fill-distance near the boundary, which provides the flexibility to create the domain by manually digitizing of the geometry from a hand-drawing, digital-drawing, or a downloaded image." * [x] Can you add a References heading?
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
@kyleniemeyer @vijaysm : The release version is v1.0 after incorporating @Kevin-Mattheus-Moerman 's suggestions.
@pankajkmishra
Thanks for implementing most of the points. Below are some remaining points on the paper:
[x] This part has not been changed yet: "discrete set of point cloud". Should this be "discrete sets of point clouds" or "discrete point cloud sets" ? Also "which need not to be" sounds odd, I suggest you remove the word "to".
[x] Should the long hyphen be removed before state of the art? "... (and keep) it -state-of-the-art by incorporating..."
@pankajkmishra thanks for expanding the demo file.
Minor point, perhaps update this comment:
% You can add similar visualization section for other demos, or your own experiment
% to have a better understanding of NodeLab.
To:
% You can add similar visualization sections to other demos or your own code
% to have a better understanding of the inputs used in NodeLab.
@pankajkmishra about the license. The GNU license you've chosen, strictly speaking, requires that you add the following "boiler plate text" under all your codes (see also: https://opensource.org/licenses/GPL-3.0):
<one line to give the program's name and a brief idea of what it does.>
Copyright (C) <year> <name of author>
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
I recommend you update this text with your information and paste it at the end of your codes. If you want to automate this, check out how I did this for my personal project GIBBON. I use an automated script to remove or add these boiler plates to the bottom of selected functions (see addCodeFooter.m and removeCodeFooter.m)
@pankajkmishra I've left the "automated test" box above unticked. I think manually running the codes in the demos folder is fine, so the following is a recommendation rather than a requirement.
To add a basic type of automated test functionality, I recommend you just create a single script called testNodeLab
in the main folder which runs all the demos. (I have something similar, but a bit more complicated, for GIBBON here: https://github.com/gibbonCode/GIBBON/blob/master/lib/testGibbon.m).
@Kevin-Mattheus-Moerman : Done
* [x] This part has not been changed yet: "discrete set of point cloud". Should this be "discrete sets of point clouds" or "discrete point cloud sets" ? Also "which need not to be" sounds odd, I suggest you remove the word "to". * [x] Should the long hyphen be removed before state of the art? "... (and keep) it -state-of-the-art by incorporating..."
@Kevin-Mattheus-Moerman Done
% You can add similar visualization section for other demos, or your own experiment % to have a better understanding of NodeLab.
To:
% You can add similar visualization sections to other demos or your own code % to have a better understanding of the inputs used in NodeLab.
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
Nearly there with the paper. I meant to remove the hyphen altogether, so use "state-of-the-art" instead of "-state-of-the-art".
Nearly there with the paper. I meant to remove the hyphen altogether, so use "state-of-the-art" instead of "-state-of-the-art".
Done.
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
@pankajkmishra I've left the "automated test" box above unticked. I think manually running the codes in the demos folder is fine, so the following is a recommendation rather than a requirement.
To add a basic type of automated test functionality, I recommend you just create a single script called
testNodeLab
in the main folder which runs all the demos. (I have something similar, but a bit more complicated, for GIBBON here: https://github.com/gibbonCode/GIBBON/blob/master/lib/testGibbon.m).
I have added a simple "automated test", which sets up the path and runs all the tests in the demos directory one-by-one. Check it out.
@pankajkmishra about the license. The GNU license you've chosen, strictly speaking, requires that you add the following "boiler plate text" under all your codes (see also: https://opensource.org/licenses/GPL-3.0):
<one line to give the program's name and a brief idea of what it does.> Copyright (C) <year> <name of author> This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with this program. If not, see <http://www.gnu.org/licenses/>.
I recommend you update this text with your information and paste it at the end of your codes. If you want to automate this, check out how I did this for my personal project GIBBON. I use an automated script to remove or add these boiler plates to the bottom of selected functions (see addCodeFooter.m and removeCodeFooter.m)
@Kevin-Mattheus-Moerman Since NodeLab is a small repository, I added these lines manually. Thanks for the automated scripts though, could come in handy, in future.
@pankajkmishra can you describe in more detail what advances/novelty your work offers over the work (and MATLAB script) by Fornberg & Flyer. If one glances at your project it seems, as stated in the paper, that the core of the matter relies on node_places
which appears to stem rather directly from the Fornberg & Flyer implementation, and that on top of that your library contains about 6 small scripts containing between 5-15 lines of code which appear to simply call the external codes from the ext folder. Therefore, in order to understand/determine if this work is a significant advance on those published works, can you describe what changes you've made and what technical novelty you've added/enabled?
@Kevin-Mattheus-Moerman : The main idea is to integrate the node_placing algorithm in a geometric modeling framework like FEM mesh-generators.
I am using Fornberg & Flyers approach for initial node-generation in the bounding box (rectangle) and then using the geometric modeling method similar to what (Persson & Strang, 2004) do for generating FEM meshes - but not using the iterative (time-consuming part) refinement, because meshless methods does not need as perfect nodes as FEM. I am using an external C++ code for creating polygon with point-clouds and a curve interpolation function to keep the user defined density of boundary nodes. I, initially, had written this for my own numerical tests and thought that it might be useful to others working in meshfree community.
As far as the value addition over (Forberg & Flyer) is concerned - If you see page 24 - Table 2 (first column) in (https://arxiv.org/pdf/1812.03160.pdf) they mention a few limitations of Fornberg & Flyer (FF) approach
(1) FF supports partial irregular domains while here we can create nodes in any kind of irregular domains.
(2) FF lacks compatibility with the boundary nodes - present work does not (providing manual control)
(3) FF does not guarantee 'minimal spacing' between nodes - however in the present work we ensure the minimal spacing through the variable 'ptol'.
I had the feeling from JOSS author guidelines that I shouldn't mention these details in the main paper.
@Kevin-Mattheus-Moerman @kyleniemeyer : If this work does not fit in JOSS's aim and scope, feel free to let me know. I'd be fine to keep it 'just as a github repository', and others can still use it, which was the original purpose.
@Kevin-Mattheus-Moerman does the explanation above satisfy your concerns?
@pankajkmishra It may be helpful to specifically differentiate your work from the F&F paper and mention how you build on that. Also, any code in ext
that came from somewhere else should be cited in the paper.
@kyleniemeyer I am waiting for @Kevin-Mattheus-Moerman 's response.
@pankajkmishra Please document that the statistics toolbox is a required dependency in order to compute local pairwise distances between points. Instead, this dependency can be easily removed by exposing your own pdist2
implementation like below.
function [d]=pdist2loc(cpts,xyp)
d=sqrt((cpts(:,1)-xyp(:,1)).^2+(cpts(:,2)-xyp(:,2)).^2);
end
and then using pdist2loc
instead of pdist2
in the tests:
radius = @(p,ctps) 0.005+0.08*(min(pdist2loc(ctps, p))); % for variable node-density
@pankajkmishra It would also be useful to talk about future extensions to 3-D domains and a simple tessellation algorithm with the node placement such that an unstructured mesh can be generated with user-specified refinement criteria. This would improve the application of the NodeLab software to not just meshfree solvers, but even for adaptive FEM simulations. This is more of a suggestion for adding a TODO file to the repository with some extension ideas.
@kyleniemeyer in relation to
@Kevin-Mattheus-Moerman does the explanation above satisfy your concerns?
Yes, somewhat. My key "concern" is that this submission is in my opinion borderline in terms of what it adds on top of previous work, and very close to being too minor a contribution. This work appears to largely be a collection of codes (with minor changes) developed by others, combined with a small amount of novel codes. Despite the above, I have ticked all the boxes, since I do feel this is a very useful project, and the author has added useful examples and demos. If the editor @kyleniemeyer and other reviewer @vijaysm feel favourable about this submission I am happy to accept this work.
Thank you @vijaysm - I'll edit the repository according to your suggestions. I have already mentioned 3D in the future development. However, I am not targeting FEM users at the moment.
I do agree with @Kevin-Mattheus-Moerman remark about using mostly external codes. I could have rewritten those codes (e.g. FF's node placing, or desegment.cpp) and added it to the library, which would, probably, require extra efforts from a user (who is already familiar with the external codes I have used) to understand my repository.
I understand that it looks "borderline" in terms of code writing, but I do believe (as the first user) that it is useful to the target users because of the end results, which is not that "borderline". Each external, and internal code other than "node_placing.m" adds something to it. Moreover, The fact that there is a lack of open-source repository for meshfree node-generation makes this repository useful at the moment.
@kyleniemeyer I have cited all the external sources. I'll wait for your decision on this and then do the final polishing to the repository.
Best regards,
@vijaysm β What is your assessment with regards to the issues raised by @Kevin-Mattheus-Moerman? What about you, @kyleniemeyer? The author has received no reply for 23 days. Do you feel this can go on to publication, or is the "minor contribution" assessment a blocker?
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
@labarba if @vijaysm favors accepting, then I also will give my recommendation to move forward (pending some editorial feedback on the paper)
@labarba @kyleniemeyer I agree with @Kevin-Mattheus-Moerman that the contribution is an incremental update to the work by Fornberg [1] in terms of the algorithmic aspects. However, I feel that NodeLab exposes the node-generation infrastructure in a general setting that is directly usable by solvers written in MATLAB. Extensions to 3-D would have made a stronger case for acceptance of the current submission, especially given the fact that a new preprint by Sande and Fornberg is now available on Arxiv [2].
Given the potential for this contribution to support practical use-cases, I can provide a conditional acceptance if the following comments are addressed:
[1] Fornberg, B., Flyer, N. (2015). Fast generation of 2-D node distributions for mesh-free PDE discretizations. Computers & Mathematics with Applications, 69(7), 531-544.
[2] Kiera van der Sande, Bengt Fornberg, Fast variable density 3-D node generation, https://arxiv.org/abs/1906.00636.
ok @pankajkmishra, sorry for the delay here, but could you address @vijaysm's comments?
Thank you! I'll respond to @vijaysm 's comments ASAP. I have switched my job and moved from Hong Kong to the US, so I might need a week or two to draft my response and make some changes.
@pankajkmishra no worriesβI'm going to set an automated reminder for another week from now.
@whedon remind @pankajkmishra in 1 week
Reminder set for @pankajkmishra in 1 week
:wave: @pankajkmishra, please update us on how things are progressing here.
@whedon I am on it. Will respond soon.
Hi @pankajkmishra, just checking in. Have you been able to make progress on this?
Hi @kyleniemeyer : I'll finish it this week. I have finally settled in my new job in the US after Hong Kong, which is the reason for the delay from my side.
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
@kyleniemeyer I have updated the paper showing better a clarity for the value addition.
Added the statistics toolbox dependency as @vijaysm had suggested. However, I couldn't make a similar 3D extension of it at the moment and it continues to be in the future work section. If the present form of the repository does not qualify to be listed in JOSS, I would be happy to withdraw the submission :)
P.S. I have a paper under review where I need to cite this work, which was the original purpose of submitting this to the JOSS.
Thanks @pankajkmishra.
@vijaysm, do you recommend acceptance based on the changes made?
@pankajkmishra Thanks for the description update.
@kyleniemeyer While the updated paper makes the application space clearer than before, I would like to reiterate that the work on NodeLab still remains incremental. Node generation support for 3-D volumes or surfaces would certainly compel me to accept the paper. Also, other open-source Matlab packages by Per-Olof Persson (Distmesh [1]) and by Darren Engwirda (Mesh2D [2]), already do provide adaptive node generation and delaunay triangulation based on user-defined distance functions as well for 2-D domains. Additionally, I think some NodeLab examples showing adaptive refinement with integration to a mesh-free solver would have added value to this contribution with a practical use-case.
As it stands, I am taking the decision to reject the paper submission. @pankajkmishra, please consider updating examples and adding support for 3-D domains to differentiate this work from other existing open-source alternatives. I would be happy to discuss my comments further if you have any questions.
[1] http://persson.berkeley.edu/distmesh/
[2] https://github.com/dengwirda/mesh2d
Hi @vijaysm, thanks for your input.
I should point out that JOSS does not reject submissions for novelty, originality, or impact; instead, we only reject when the authors are unwilling to improve the software to meet our standards. Since both you and @Kevin-Mattheus-Moerman have checked off all the review items, and it doesn't seem like there are any remaining items of that nature, I am going to move this into the final acceptance stages.
@pankajkmishra at this point please archive the software repository (e.g., in Zenodo) and provide the DOI here. I'm going to do a final review/edit of the paper itself.
@kyleniemeyer Fair enough. In that case, the description update from @pankajkmishra was all that was needed to make the submission complete for me.
Hey @kyleniemeyer , thanks! I'll do it ASAP.
Thank you @vijaysm @Kevin-Mattheus-Moerman for your valuable inputs.
@kyleniemeyer following is the Zenodo DOI
10.5281/zenodo.3361734
@whedon set 10.5281/zenodo.3361734 as archive
OK. 10.5281/zenodo.3361734 is the archive.
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
Hi @pankajkmishra , just a few final edits needed on the paper:
There are a few more places in the paper that should be edited:
The literature in this context is up-and-coming, which focus on different aspects of node-generation based on typical requirements. The node-placing approach by @Fornberg:2015 is similar to advancing front methods and has been reported to have advantages like computational speed, simple algorithms, and good quality of distribution.
- The first sentence here doesn't make sense to meβI'm not sure what it's actually saying.
- Anytime you reference "literature", it should be followed by multiple citations. Also, what are the "typical requirements"?
- You mention "advancing front methods", but provide no definition or citation. Why is the fact that the node-placing approach is similar to these methods meaningful?
Aside from that, the first paragraph should start with a statement of need, meant for a general reader. I think you could move your sentence about the applications here, and do some revision to make it work.
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
@pankajkmishra π
@openjournals/joss-eics ok, this is ready for acceptance from my point of view
Thanks @kyleniemeyer , and everyone :)
@whedon accept
Attempting dry run of processing paper acceptance...
Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/903
If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/903, then you can now move forward with accepting the submission by compiling again with the flag deposit=true
e.g.
@whedon accept deposit=true
Thanks to @Kevin-Mattheus-Moerman and @vijaysm for reviewing and @kyleniemeyer for editing
@whedon accept deposit=true
Doing it live! Attempting automated processing of paper acceptance...
π¦π¦π¦ π Tweet for this paper π π¦π¦π¦
π¨π¨π¨ THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! π¨π¨π¨
Here's what you must now do:
Party like you just published a paper! πππ¦ππ»π€
Any issues? notify your editorial technical team...
:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:
If you would like to include a link to your paper from your README use the following code snippets:
Markdown:
[](https://doi.org/10.21105/joss.01173)
HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.01173">
<img src="https://joss.theoj.org/papers/10.21105/joss.01173/status.svg" alt="DOI badge" >
</a>
reStructuredText:
.. image:: https://joss.theoj.org/papers/10.21105/joss.01173/status.svg
:target: https://doi.org/10.21105/joss.01173
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:
Most helpful comment
@kyleniemeyer Fair enough. In that case, the description update from @pankajkmishra was all that was needed to make the submission complete for me.