search

openjournals / joss-reviews

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

[REVIEW]: DEVSIM: A TCAD Semiconductor Device Simulator #3898

Closed whedon closed 2 years ago

whedon commented 2 years ago

Submitting author: @tcaduser (Juan E. Sanchez) Repository: https://github.com/devsim/devsim.git Version: v2.0.1 Editor: @lucydot Reviewer: @jgoldfar, @dalonsoa Archive: 10.5281/zenodo.5908666

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

Status

status

Status badge code:

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

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

@jgoldfar & @dalonsoa, 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 @lucydot 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 @jgoldfar

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @dalonsoa

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 2 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @jgoldfar, @dalonsoa 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/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 2 years ago

PDF failed to compile for issue #3898 with the following error: 

 Can't find any papers to compile :-(
whedon commented 2 years ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.44 s (1353.8 files/s, 205502.8 lines/s)
----------------------------------------------------------------------------------
Language                        files          blank        comment           code
----------------------------------------------------------------------------------
C++                               195           8695           4117          42382
C/C++ Header                      194           3372           3045           8817
Python                             90           1423           2319           4529
yacc                                3            148             96           1500
Fortran 77                         15              3           2606           1495
CMake                              32            179            134           1012
Bourne Shell                       37            214            252            859
GLSL                                6             70             32            657
lex                                 3            134             50            475
Markdown                            4            127              0            198
Tcl/Tk                              2             28             41            103
Windows Message File                5             27              0             95
YAML                                2             54            127             88
DOS Batch                           1             11              0             40
C Shell                             1              1              0              7
----------------------------------------------------------------------------------
SUM:                              590          14486          12819          62257
----------------------------------------------------------------------------------

Statistical information for the repository '230ca5d69b3653164f198d9b' was
gathered on 2021/11/08.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Juan                             2         65898              0           49.84
Juan E Sanchez                   1            57             48            0.08
Juan E. Sanchez                162         39049          26658           49.70
Juan Sanchez                     3           408             94            0.38

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
Juan E. Sanchez           78699          201.5         14.6               13.33
lucydot commented 2 years ago

@whedon generate pdf from branch publications

whedon commented 2 years ago 
Attempting PDF compilation from custom branch publications. Reticulating splines etc...
whedon commented 2 years ago

PDF failed to compile for issue #3898 with the following error: 

 Can't find any papers to compile :-(
lucydot commented 2 years ago

@whedon generate pdf from branch joss

whedon commented 2 years ago 
Attempting PDF compilation from custom branch joss. Reticulating splines etc...
whedon commented 2 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 2 years ago

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

whedon commented 2 years ago

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

dalonsoa commented 2 years ago

It's in my ToDo list for this week. Expect an updated by the weekend.

dalonsoa commented 2 years ago

I have a problem with testing.

devsim includes a collection of python scripts that run as part of its testing suite and that check the different functionality. However, it uses no standard testing framework for the high level python interface (pytest, unittest) nor a unit testing framewrok for the low level c++ functinality (catch). What there is now is neither exaustive nor sustainable.

However, requiring to implement any of this is a major and very time consuming undertaking and I am reluctant to block the publication until it is done.

@lucydot , what si your suggestion on this?

tcaduser commented 2 years ago

Hello,

The tests are driven via CTest and are run as part of the release process. For each platform, the regressions are run and new results are updated before each release.

These are the existing regression tests: https://github.com/devsim/devsim_tests_msys https://github.com/devsim/devsim_tests_win64 https://github.com/devsim/devsim_tests_macos_gcc_x86_64 https://github.com/devsim/devsim_tests_linux_x86_64

This includes the tests in the testing and examples in the examples directory.

dalonsoa commented 2 years ago

About the paper:

The description of the State of the field is a bit limited. It mentions two open source packages that operate in the same field, but it does not make any comparison with them in terms of features or capabilities. More importantly, it does not compare with the very strong comercial packages (SILVACO, COMSOL). Obviously, one of the benefits of devsim is that is is open source and free compared with the $$$$ that the comercial solutions cost, so I think there is a strong case to be made on how this open source tool can leverage the functionality of some of the other, way less accessible, commercial packages.

tcaduser commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

PDF failed to compile for issue #3898 with the following error: 

 Can't find any papers to compile :-(
tcaduser commented 2 years ago

@whedon generate pdf from branch joss

whedon commented 2 years ago 
Attempting PDF compilation from custom branch joss. Reticulating splines etc...
whedon commented 2 years ago

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

tcaduser commented 2 years ago

@dalonsoa

About the paper:

The description of the State of the field is a bit limited. It mentions two open source packages that operate in the same field, but it does not make any comparison with them in terms of features or capabilities. More importantly, it does not compare with the very strong comercial packages (SILVACO, COMSOL). Obviously, one of the benefits of devsim is that is is open source and free compared with the $$$$ that the comercial solutions cost, so I think there is a strong case to be made on how this open source tool can leverage the functionality of some of the other, way less accessible, commercial packages.

Thank you for your insight. The TCAD landscape is very broad. Since I have not used the other OSI-compliant open source offerings, I do not feel it is fair to comment on their capabilities. Especially in that both Solcore and Charon are subject to rapid change. Cogenda TCAD (GPL3) has not made any updates to their software in the recent future.

As for the commercial offering, I would find it difficult to compare this tool directly to the offerings of Synopsys, Silvaco, and COMSOL. From what I can tell, the first two are comprehensive solutions with person decades of experience. COMSOL is an FEM multiphysics package, that made the extra effort to implement FVM to incorporate Scharfetter Gummel.

I would prefer to not even get started discussing the tools with restrictive open/closed-source academic licenses. Stanford's PISCES, Glasgow's Open NESS, and Florida's FLOODS come to mind.

How much detail would you want for this section? I have added an additional citation to the paper for a researcher using DEVSIM for DLTS.

lucydot commented 2 years ago

RE: testing - @dalonsoa are the queries you raised partly answered by the response from @tcaduser (which seems to relate to a C++ testing unit framework) or are you still hoping for more clarification from editorial point of view?

Ideally the Python interface would have automated testing using pytest or similar, however if there are tests that a user can run manually then this is acceptable.

@tcaduser It may be that you need to clarify details around testing in the documentation (indicate coverage, what is automated, what is manual, how a user can verify that the installation is correct and the software is working as expected).

jgoldfar commented 2 years ago

👋 @jgoldfar, please update us on how your review is going (this is an automated reminder).

I am going through the steps in this listing in this issue, and have found that I cannot toggle the "check boxes" - is this a problem on my end, or is it because I am not assigned to this issue?

lucydot commented 2 years ago

Hi @jgoldfar that's weird - you are listed as a reviewer so should have been assigned automatically. I'll try adding you now. Thanks for flagging this up.

lucydot commented 2 years ago

Ok, I've added you manually, let's see if it works!

tcaduser commented 2 years ago

@jgoldfar, @dalonsoa

I have merged the joss branch with the r1.7.0 branch for the latest versions of INSTALL.md, README.md, . . . The pdf form of the documentation is in doc/devsim.pdf of the same branch.

tcaduser commented 2 years ago

@jgoldfar, @dalonsoa

The newest release branch r2.0.0 has been merged into the joss branch.

dalonsoa commented 2 years ago

@tcaduser , I'm happy with the improvements in the installation instructions - great job! - and I've also checked some features and performance. All good on that front. After the clarification above, I am also happy with the testing part - although I strongly recomend opening an issue about implementing a unit testing suite - for both the C++ part and the Python part. But that is work for the future.

Now, I am still not convinced by the Statements of Purpose in both the paper and the documentation, as well as the State of the Field. I understand the arguments you give above, but even that discussion - without actually comparing features, something I agree is very challenging - is extremelly valuable to be written down as part of the documents. This way, people can easily put DEVISIM on the map of TCAD software, which is an enormous field.

Also, the Contributing guidelines. At the very least, I would describe the whole ecosystem of repositories you have in place hosting the source code, the tests for the different platforms, etc, so potential contributors cand find out how things are arranged.

In summary: the tool itself is great, but it is necessary some extra work to clarify where it sits in the big scheme of things and how to support it.

tcaduser commented 2 years ago

@whedon generate pdf from branch joss

whedon commented 2 years ago 
Attempting PDF compilation from custom branch joss. Reticulating splines etc...
whedon commented 2 years ago

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

tcaduser commented 2 years ago

@dalonsoa please check the updated paper, to see if it meets your expectations. It adds an extended discussion comparing the simulator to the commercial offerings. It also sets a list of requirements for the class of TCAD problems it solves. The paper now explicitly references SYMDIFF, the symbolic differentiation engine that is used by the simulator, and that is also maintained by our project.

On the joss branch The CONTRIBUTING.md now points back to the README.md, where I have explicitly added the related projects.

danielskatz commented 2 years ago

@whedon re-invite @jgoldfar as reviewer

whedon commented 2 years ago

The reviewer already has a pending invite.

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

lucydot commented 2 years ago

@dalonsoa @jgoldfar @tcaduser please note that I am setting my out of office for the next two weeks over Christmas. I will check in once a week, but my responses might be slower than usual for the next fortnight.

@jgoldfar please let me know if you are still having problems with the tick-boxes, there is an invite link just above this message.

lucydot commented 2 years ago

/ooo December 20 until January 3

dalonsoa commented 2 years ago

@tcaduser , now this looks really, really good, both the paper and the new README. Many thanks for taking the time to implementing the changes.

All good from my side. After merging the JOSS branch, I recomend accepting the work for its publication in JOSS.

lucydot commented 2 years ago

@dalonsoa thank you for your timely and thoughtful review.

We are waiting on a second review from @jgoldfar - could you give us an update on how this is going? I will also contact through email as I know sometimes the github notifications are missed (I am guilty of this anyhow).

lucydot commented 2 years ago

@whedon re-invite @jgoldfar as reviewer

whedon commented 2 years ago

OK, the reviewer has been re-invited.

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

jgoldfar commented 2 years ago

Thanks @lucydot - I'm able to check off the tasks for this review now.

@tcaduser - well done on this comprehensive work. I would support a more automated approach to testing as suggested by @dalonsoa for the continued robustness of your project; it looks like you have integration tests as mentioned in your comment, though they don't appear to be automated. Unit testing for every component of your application is probably too much to ask, and can create unnecessary fragility - you are probably the best person to decide on right combination of unit tests and automated functional or integration tests.

Recognizing that the technical aspects of the work are more of a journey than a destination, I too recommend accepting this work for publication in JOSS.

lucydot commented 2 years ago

Hi @jgoldfar that's great - thank you for your comments. If you are happy that the software passes criteria, can I ask you to tick off the appropriate boxes at the top of the issue? There are two unticked. Note that the "automated testing" can be ticked if there are manual steps outlined so that a user can verify that the software is working as intended - which I believe, from the discussions I've seen, is the case here.

jgoldfar commented 2 years ago

Done

lucydot commented 2 years ago

@whedon generate pdf from branch joss

whedon commented 2 years ago 
Attempting PDF compilation from custom branch joss. Reticulating splines etc...
whedon commented 2 years ago

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

lucydot commented 2 years ago

@whedon check references

lucydot commented 2 years ago

On behalf of the JOSS editorial team, thank you very much @jgoldfar and @dalonsoa for your reviews ✨

@tcaduser it looks like we are onto the final lap!

I have a few editorial suggestions for the JOSS article - to improve readability:

Once you make the necessary changes please let me know here - thanks!

tcaduser commented 2 years ago

@whedon generate pdf from branch joss

whedon commented 2 years ago 
Attempting PDF compilation from custom branch joss. Reticulating splines etc...