jakelangham commented 7 months ago

I am looking over your JOSS paper preprint as part of https://github.com/openjournals/joss-reviews/issues/6288. Here are a couple of small things I noticed regarding the MATLAB Poisson solver example given.

If the example is to be included, it should be possible to read from the text (not the code) exactly what the code snippet will do. This would look something like: defining the problem exactly, giving the right-hand side and boundary conditions and stating that the code approximates the solution to the problem by constructing the corresponding 4th order mimetic operator and directly inverting it.
Likewise, the figure caption ought to be self-contained, or at least reference an equation where the problem is defined. Note that BVP should be defined since it won't count as a standard abbreviation for this journal.
The example code doesn't actually reproduce the figure exactly because the analytical solution isn't plotted.
While a 2D Poisson solver is given in examples_MATLAB/, I don't see this 1D code. I think it should be added there since this is the first example that most new users will likely want to look at.

jcorbino commented 6 months ago

Thanks, @jakelangham - I will make the suggested changes.

jcorbino commented 4 months ago

@jakelangham I made the following minor changes:

1 Removed BVP and referred to the problem as 1D Poisson eqn with Robin BCs.

2 Added comment line to code snippet in the draft with the file's name (elliptic1D.m).

3 The order of accuracy in the example (.m) file was changed to 4 (it was 6) to align with the paper's consistency and to avoid potential confusion.

4 I intentionally omitted the analytical solution's plotting since it wasn't interesting for the snippet. If this can be confusing, I'll gladly add the line that plots the analytical or remove the analytical from the picture.

Please let me know if these changes are enough to fulfill the requirements in this issue so I can proceed to close it.

Thank you,

jakelangham commented 4 months ago

@jcorbino Looks good to me. However, I don't see the filename elliptic1D.m referenced in the new draft. Perhaps it was left out unintentionally (unless I've just missed it)?

jcorbino commented 4 months ago

@jakelangham My mistake. I forgot to regenerate the PDF using the editorialbot generate pdf command. I observed the changes in the artifact produced by the workflow. I simply added a comment line to the snippet: % File: elliptic1D.m

capture

jakelangham commented 4 months ago

Ok - in that case I think this can be closed.

jcorbino / mole

[joss review] 1D Poisson example #11

1 Removed BVP and referred to the problem as 1D Poisson eqn with Robin BCs.

2 Added comment line to code snippet in the draft with the file's name (elliptic1D.m).

3 The order of accuracy in the example (.m) file was changed to 4 (it was 6) to align with the paper's consistency and to avoid potential confusion.

4 I intentionally omitted the analytical solution's plotting since it wasn't interesting for the snippet. If this can be confusing, I'll gladly add the line that plots the analytical or remove the analytical from the picture.