Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JOSS Submission Review (Reviewer 2): Software paper #10

Closed
15 tasks done
damar-wicaksono opened this issue Aug 13, 2024 · 7 comments
Closed
15 tasks done

JOSS Submission Review (Reviewer 2): Software paper #10

damar-wicaksono opened this issue Aug 13, 2024 · 7 comments

Comments

@damar-wicaksono
Copy link

damar-wicaksono commented Aug 13, 2024

This issue is related to the JOSS submission review.

Below are my remarks and suggestions regarding the software paper. Just let me know if further clarifications are need. Thank you!

Summary

  • I think the first paragraph of the next section ("Statement of need") should be switched as the "Summary" of the paper as it better describes the high-level functionality and purpose of the package. Furthermore, the summary should also include the target audience of the package which the authors have also stated in the last paragraph of the next section.
  • "...more hostile environment compared to...": Perhaps simply "than" is better than "compared to".
  • "...and able to combine together different sources...": I think "together" is unnecessary.
  • "...and real data (i.e., measurements)...": Perhaps to be succinct, this can be written simply as "measurement data"
  • "...the dynamics of un-observable complex field...": Somewhere else in the paper "non-observable" is used, perhaps simply pick one?

Statement of need

  • As noted above, I think the first paragraph should be switched with the paragraph in "Summary". The paragraph in Summary better describes the need that is served by the package.
  • Because pyforce is an abbreviation perhaps the expanded term should appear directly after "pyforce"
  • "...mainly for the Nuclear Engineering world.": Perhaps "in" instead of "for"
  • "...Nuclear Engineering...": Is the capitalization really necessary?
  • "The package is part of the ROSE...": Is there a reference to ROSE perhaps a link or a document?
  • "at reducing..., at searching... and at integrating...": I think there is no need to repeat the preposition "at" due to already parallel structure
  • Figure 1 should be better clarified by the second paragraph in this section; the figure contains mathematical symbols and notations that should be defined briefly and explicitly in the passage (if these symbols and notations are kept in the end). For instance, reading the paragraph it is not clear the starting point of the whole scheme (is it the field according to the full order model, or something else?).
  • I would suggest that the list of implemented features are rewritten (perhaps in tabular form) to correspond to the figure better; which ones belong specifically to the offline and online phase, just the offline phase, or just the online phase. The list structure gives the impression that these features serve similar exchangeable purposes.
  • "...either regularised with Tikhonov or not...": Perhaps it would be better to write as "either with or without Tikhonov's regularisation"
  • "...for the Online Phase": Before there was no capitalization for "Online Phase". Is the capitalization intentional?

Contribution and authorship

Currently, the paper list three authors, namely S. Riva, C. Introini, and A. Cammi. Looking at the commit contributions page, I've seen two contributors have made commits. As @Steriva is S. Riva so the question regarding "Contribution and Authorship" is settled. Is Neko-tan also one of the authors?

Related to the paper, I would like to also suggest including a list of the contributions of each author separately in the paper.
Some JOSS papers (for example, here) adopted the CRediT taxonomy to clarify those roles.
Perhaps the authors would consider adding such section to the paper?

@Steriva
Copy link
Contributor

Steriva commented Aug 13, 2024

Thank you @damar-wicaksono for the insightful comments (including #8 and #9).
In the coming weeks, I'll update the repository following your suggestions.

Just a quick note about the authors: @Neko-tan is Carolina Introini, whereas A. Cammi is the professor supervising all the activities: I'll add the CRediT taxonomy to the paper to clarify the roles.

Thank you again!

@Steriva
Copy link
Contributor

Steriva commented Sep 3, 2024

Response Letter: Software Paper

Thank you very much for the useful comments and suggestions. Here are listed the response and the changes made to the joss-paper.

The changes are reported in pull request #12.

Summary

  1. I think the first paragraph of the next section ("Statement of need") should be switched as the "Summary" of the paper as it better describes the high-level functionality and purpose of the package. Furthermore, the summary should also include the target audience of the package which the authors have also stated in the last paragraph of the next section.
    • The paragraphs have been switched and more insights about the target audience of the package are added.
  2. "...more hostile environment compared to...": Perhaps simply "than" is better than "compared to".
    • Fixed
  3. "...and able to combine together different sources...": I think "together" is unnecessary."
    • Fixed
  4. "...and real data (i.e., measurements)...": Perhaps to be succinct, this can be written simply as "measurement data"
    • Changed to measurements data (i.e., local evaluations of quantities of interest).
  5. "...the dynamics of un-observable complex field...": Somewhere else in the paper "non-observable" is used, perhaps simply pick one?
    • Fixed.

Statement of need

  1. Because pyforce is an abbreviation perhaps the expanded term should appear directly after "pyforce"
    • Changed
  2. "...mainly for the Nuclear Engineering world.": Perhaps "in" instead of "for"
    • Fixed
  3. "...Nuclear Engineering...": Is the capitalization really necessary?
    • No, it isn't: changed
  4. "The package is part of the ROSE...": Is there a reference to ROSE perhaps a link or a document?
    • Some comments about ROSE (an acronym used within the ERMETE-Lab to refer to ROM related applications to multi-physics for nuclear reactors).
  5. "at reducing..., at searching... and at integrating...": I think there is no need to repeat the preposition "at" due to already parallel structure
    • Fixed
  6. Figure 1 should be better clarified by the second paragraph in this section; the figure contains mathematical symbols and notations that should be defined briefly and explicitly in the passage (if these symbols and notations are kept in the end). For instance, reading the paragraph it is not clear the starting point of the whole scheme (is it the field according to the full order model, or something else?).
    • The description of the Figure has been greatly expanded, the Figure itself has been updated to reflect the one present on the README of the Github repo.
  7. I would suggest that the list of implemented features are rewritten (perhaps in tabular form) to correspond to the figure better; which ones belong specifically to the offline and online phase, just the offline phase, or just the online phase. The list structure gives the impression that these features serve similar exchangeable purposes.
    • Implemented
  8. "...either regularised with Tikhonov or not...": Perhaps it would be better to write as "either with or without Tikhonov's regularisation"
    • Changed
  9. "...for the Online Phase": Before there was no capitalization for "Online Phase". Is the capitalization intentional?
    • No, it isn't: changed

Contribution and authorship

CRediT taxonomy has been added to clarify the roles, reported below.

  • Stefano Riva: Conceptualization, Data curation, Formal analysis, Software, Visualization, Writing – original draft
  • Carolina Introini: Conceptualization, Formal analysis, Software, Supervision, Writing – review & editing
  • Antonio Cammi: Conceptualization, Project administration, Resources, Supervision, Writing – review & editing

@damar-wicaksono
Copy link
Author

@Steriva: Thank you very much for taking the time going through my comments! So that I can verify the changes and close this issue, is the updated draft can already be built into a PDF file from the JOSS review page? I'm asking because it seems that the pull request is still open...

@Steriva
Copy link
Contributor

Steriva commented Sep 10, 2024

I've just merged the pull request into the main branch. Thank you very much for all the valuable comments!

@damar-wicaksono
Copy link
Author

damar-wicaksono commented Sep 13, 2024

Thank you again for revising the paper! I went through the latest draft and confirmed that my previous remarks have been addressed.

There are a couple additional things that you might want to consider before I can close this issue.

Statement of need

  • l. 33 "...whereas the latter is cheap from the computational point of view...": Why not use the previous construction of "computationally expensive" and state simply "computational cheap"?
  • l. 34 "...point of view and allows to have quick and reliable evaluations...": If I'm not wrong "allow to" needs a direct object so either "allows one to have quick" or change the preposition into "allows for quick and reliable evaluations..." or change the verb to, say, "enable".
  • l. 36 "During the offline...": Perhaps start a new paragraph here to have a clear separation from what came before (general idea of offline/online phase) and what is coming after (the online phase).
  • l. 40 "...reduced representation through a set of basis functions $\{ \psi_n(\boldsymbol{x}) \}$...": maybe add directly after the set "of size $N$", i.e., "...through a set of basis functions $\{ \psi_n(\boldsymbol{x}) \}$ of size $N$..."
  • l. 41 typo "decresed"
  • l. 41, similar comment as above regarding "allow to" without a direct object
  • l. 44 "...allows for the search of the optimal positions of sensors...in a more efficient manner": I think "allows for" is correct but it should be "search for" instead of "search of" in the present usage. So perhaps: "...allows for efficiently searching for optimal sensor placements in the physical domain."?
  • l. 46 "All these steps are performed during the offline phase,...": In my opinion this is not necessary, simply start with this new paragraph the online phase.
  • l. 46 "..., the online phase aim consists in obtaining...": Why not "the online phase aims to obtain..."
  • l. 48-49 "The DDROM online takes place which produces a novel set of reduced variables, $\alpha^*$, and then computing an improved reconstructed state $\hat{u}_{\mathrm{DDROM}}$ through a decoding step from the low dimensional state to the high dimensional one.": I find this hard to read. Perhaps as an alternative:

"The DDROM online phase produces a new set of reduced variables (are these "coefficients", as in the above, or "variables"?), $\alpha^*$, and then computes an improved reconstructed state $\hat{u}_{\mathrm{DDROM}}$ through a decoding step that transforms the low-dimensional representation to the high-dimensional one."

And with that, I have no further comments regarding on the draft manuscript. Please let me know if you need clarifications. Thank you!

@Steriva
Copy link
Contributor

Steriva commented Sep 15, 2024

Thanks for the comments: all the changes have been implemented and merged into the main branch with pull request #13.

The draft paper has been generated with this action.

@damar-wicaksono
Copy link
Author

damar-wicaksono commented Sep 16, 2024

Thank you very much for the revision! I confirm that the points I've raised have been resolved.

One additional very minor issue that you might want to correct is line 56 on page 3: "...and its scope of application is not only restricted to the Nuclear Engineering world..." which to be consistent with the previous usage should not be capitalized.

I'm closing this issue now.... Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants