[REVIEW]: OptiCommPy: Open-source Simulation of Fiber Optic Communications with Python

[editorialbot]

Submitting author: @edsonportosilva (Edson Porto da Silva)
Repository: https://github.com/edsonportosilva/OptiCommPy/
Branch with paper.md (empty if default branch): add-paper
Version: v0.9.0-alpha
Editor: @lucydot
Reviewers: @klb2, @ebezzam, @taladjidi
Archive: 10.5281/zenodo.11450597

Status

Status badge code:

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

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

@klb2 & @ebezzam & @taladjidi, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review.
First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

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 ✨

Checklists

📝 Checklist for @klb2

📝 Checklist for @taladjidi

📝 Checklist for @ebezzam

[editorialbot]

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf

[editorialbot]

Software report:

github.com/AlDanial/cloc v 1.90  T=0.23 s (1083.0 files/s, 313349.5 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
HTML                            86           9888             86          15254
JavaScript                      13           2439           2514           9272
Python                          33           2374           3718           5227
Jupyter Notebook                19              0          13819           2699
SVG                              1              0              0           2671
CSS                              4            190             40            779
reStructuredText                86            220            237            211
TeX                              1             14              0            143
Markdown                         2             38              0            103
YAML                             2              6             20             28
DOS Batch                        1              8              1             26
make                             1              4              7              9
-------------------------------------------------------------------------------
SUM:                           249          15181          20442          36422
-------------------------------------------------------------------------------

Commit count by author:

  1222	Edson Porto da Silva
    28	Adolfo Herbster
    28	edsonportosilva
     3	Sourcery AI
     2	Carlos Daniel Fontes da Silva
     1	daniel7fontes

[editorialbot]

Paper file info:

📄 Wordcount for paper.md is 958

✅ The paper includes a Statement of need section

[editorialbot]

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1109/50.622902 is OK
- 10.1109/JSTQE.2010.2044751 is OK
- 10.1109/JLT.2009.2039464 is OK
- 10.1109/JLT.2020.2996188 is OK
- 10.1109/JLT.2017.2786351 is OK
- 10.1109/JLT.2017.2662082 is OK

MISSING DOIs

- No DOI given, and none found for title: Fiber-Optic Communication Systems
- 10.1016/b978-1-78548-037-9.50001-6 may be a valid DOI for title: Digital Communications
- No DOI given, and none found for title: Robochameleon: A matlab coding framework and compo...
- No DOI given, and none found for title: OptiSystem
- No DOI given, and none found for title: VPItransmissionMaker™ Optical Systems 
- No DOI given, and none found for title: Synopsys OptSim for Optical Communication
- No DOI given, and none found for title: Optilux, the optical simulatr toolbox.
- No DOI given, and none found for title: CuPy: A NumPy-Compatible Library for NVIDIA GPU Ca...
- No DOI given, and none found for title: CUDA, release: 10.2.89

INVALID DOIs

- None

[editorialbot]

License info:

🟡 License found: GNU General Public License v3.0 (Check here for OSI approval)

[editorialbot]

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

[klb2]

Review checklist for @klb2

Conflict of interest

• I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

• I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• Repository: Is the source code for this software available at the https://github.com/edsonportosilva/OptiCommPy/?
• License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
• Contribution and authorship: Has the submitting author (@edsonportosilva) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
• Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
• Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
• Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
• Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

• Installation: Does installation proceed as outlined in the documentation?
• Functionality: Have the functional claims of the software been confirmed?
• Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
• Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
• Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

• Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
• A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
• State of the field: Do the authors describe how this software compares to other commonly-used packages?
• Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
• References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

[taladjidi]

Review checklist for @taladjidi

Conflict of interest

• I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

• I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• Repository: Is the source code for this software available at the https://github.com/edsonportosilva/OptiCommPy/?
• License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
• Contribution and authorship: Has the submitting author (@edsonportosilva) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
• Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
• Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
• Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
• Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

• Installation: Does installation proceed as outlined in the documentation?
• Functionality: Have the functional claims of the software been confirmed?
• Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
• Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
• Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

• Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
• A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
• State of the field: Do the authors describe how this software compares to other commonly-used packages?
• Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
• References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

[ebezzam]

Review checklist for @ebezzam

Conflict of interest

• I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

• I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• Repository: Is the source code for this software available at the https://github.com/edsonportosilva/OptiCommPy/?
• License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
• Contribution and authorship: Has the submitting author (@edsonportosilva) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
• Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
• Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
• Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
• Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

• Installation: Does installation proceed as outlined in the documentation?
• Functionality: Have the functional claims of the software been confirmed?
• Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
• Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
• Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

• Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
• A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
• State of the field: Do the authors describe how this software compares to other commonly-used packages?
• Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
• References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

[ebezzam]

@lucydot, @edsonportosilva below is my initial review. I realize that there is an examples folder, but is it expected to run everything or is there a notebook/script that gives an overview of the main features?

Functionality

• Installation: I would recommend putting the same installation in your README and in the documentation. You can create a README.rst file, and use that within your documentation (like this instead of creating a separate file so you don’t have to edit twice). I say this because I looked at the documentation for the installation steps; and python setup.py install didn’t work for me, but pip install . did (which was in your README).
• Functionality: it would be useful to have a section or notebook to show the different features, as I’m not sure where I would start to try things out. I realize there’s an examples folder, but it is not clear where to start.
• Performance: it would be interesting to quantify the speed-up thanks to GPU acceleration, as mentioned in Lines 61-64 of the paper.

Documentation

• A statement of need: there is none in the documentation.
• Automated tests: there are none nor manual steps to verify functionality.
• Community guidelines: there is in the README, but as mentioned above for “Installation”, it could be nice to use the README in your documentation, as I didn’t see the contribution guidelines at first.

Software paper

• A statement of need: The “Statement of need” starts great by giving the relevant context, but I feel like there’s a paragraph missing after Line 40 to explicitly say something like “OptiCommPy addresses this need by…” and who the target audience is (similar to what’s said in “Summary”).
• Quality of writing: I think Lines 41 and onward should be in a separate section, e.g. with the title “OptiCommPy Code Structure”. And another section on what example scripts/code for the different features could be useful.

Miscellaneous

• Adding .ipynb_checkpoints and .pylint.d to your .gitignore

[lucydot]

Thank you for your thoughtful review @ebezzam

I realize that there is an examples folder, but is it expected to run everything or is there a notebook/script that gives an overview of the main features?

With the immediate review process in mind, this seems like an important thing to address. Could you indicate which examples demonstrate the key software functionality @edsonportosilva? This could be useful for future users, so I suggest it is also included in the documentation.

[edsonportosilva]

Hello @lucydot @ebezzam,

Thanks for the review and the feedback! In the examples folder, we provide notebooks with simulation setups with various levels of complexity to help the user. The simplest simulation of optical transmission (kind of a 'textbook example') is provided in basic_OOK_transmission.ipynb. This notebook would be the starting point e.g. for undergrad students who are taking a first course in optical communications.

The notebook test_WDM_transmission.ipynb is a more advanced example that deals with coherent optical transmission and uses main features of OptiCommPy (coherent communications, advanced fiber optic modeling, and digital signal processing (DSP)). It is also the kind of simulation targeted by experienced users, such as researchers/engineers working with optical communications. This notebook also automatically calls for GPU processing, if a suitable GPU is available. Thus, by simply running it in e.g. Colab with and without a GPU accelerator, the user can estimate how fast a GPU speeds up the simulation (for this kind of simulation, the difference is quite significant, at least 10x faster I would expect).

The other notebooks are mostly simulation scenarios to "test" various features of the code. In particular, we have included the notebook test_metrics.ipynb where all relevant functions that calculate performance metrics are tested and the results compared with results from communications theory. We are working on setting up proper automated tests for the code in the repository, but it is working in progress.

@ebezzam @lucydot, I will proceed in fixing the issues listed in your preliminary review.

[lucydot]

@edsonportosilva - thanks for the update.

As @ebezzam has already mentioned, it would be useful to have documentation to show / walk through the different features as it is difficult to know where to start with the examples folder. Often I see this as a (series of) notebooks displayed in ReadTheDocs. For example, a "Getting Started" code walkthrough showing the basic functionality. You could then link to the examples folder for the more advanced topics. This could also go some way to ensuring that the "Testing" JOSS criteria is met. We strongly encourage this would be a suite of automated tests, however the following can also be deemed acceptable:

Documented manual steps that can be followed to objectively check the expected functionality of the software (e.g., a sample input file to assert behaviour)

From here.

[taladjidi]

@lucydot @ebezzam

Sorry for the late reply, I was in leave. Below are my first comments.

Functionality

• Installation went without trouble, though I only tried through pip install .

• Examples: some of the notebooks do not run (while I did not test them all, basic_EDFA.ipynb seems to use an old package structure and does not run. I ran this first since I use an EDFA in the lab and it was the first example in the directory.

• Performance: I agree with @ebezzam that benchmarks curves would be a welcome addition. This could maybe be one of the tests ?

• GPU models / Performance: I don't know how much performance is critical, but modelsGPU.py could benefit greatly from ad hoc kernels, especially for your split step scheme. See for instance the docs from Cupy or Numba. This will help you limit the number of kernels launched. While this may be not critical for 1D problems involved with fibers, this becomes critical in 2D (which might prove relevant depending on the number of modes involved).
  Furthermore, you will gain at least a factor of 2 performance on your FFT's if you explicitely define and cache the FFT plans (see get_fft_plan or directly through cupy.cuda.cufft) instead of using the cupyx.scipy.fft API.

• Performance / FFT: I would advise switching to a more performant FFT library than Numpy's since most of you DSP will rely on it. There are a lot of potential candidates, pyFFTW could be a drop in replacement. In the same vein, a lot of operations could be parallelized and JIT compiled using Numba.

Documentation

• Examples: Jupyter notebooks are a great way to introduce the various functionalities of your software. I found that the graphical outputs were very clear. Maybe it would be good to add Markdown blocks in between the code blocks to link the code with the physical models in use (how does one equation translate in terms of your package functionalities).
• Physical models: I would expect the docstrings of the models to cite some relevant paper (when needed) explaining the model more in detail, or one or two latex equations to succintly define the used conventions etc ...
• In the paper, you compare to commercial / proprietary solutions, maybe you could include a section of your Readme to explain what funcitonalities are supported, or some kind of migration guide ? In a similar fashion, maybe you could include some performance comparisons.

Software paper

• The manuscript overall is well written and has a very good introduction.
• It would be better to split the Statement of need and the functionalities that you introduce, in order to highlight the problem on one side and its solution on the other.
• The package structure is maybe superfluous in the paper, and maybe could be replaced with benchmarks / comparisons with proprietary software (if you have access to it) ?

Overall it's a very nice project, while the examples need a bit of documentation work, they do seem to cover most of the functionalities of the package.

Best,

[klb2]

@lucydot @edsonportosilva

I have now completed my review. I tried to limit my comments to new ones that have not been covered in the reviews above.

In general, the package is interesting and contains a lot of features. The main issues are related to the tests and documentation, which lacks significant information (even missing whole modules).

Documentation

• It would be nice to add a simple example after the installation instructions to verify that the installation worked.
• In general, the documentation does not show any usage examples.
• Required version of matplotlib in documentation (>=1.4.3) does not match that in requirements/setup file (>=3.7.0)
• Pandas is not stated as a dependency in documentation
• It should be mentioned which scripts need to be run to reproduce the figures from the paper.
• The documentation of the fec module is missing in the documentation.
• Community guidelines, e.g., a CONTRIBUTING.md file, are missing

Tests

• tests/test_dsp.py does not run with the unittest package
• The setup script (setup.py) says that the nose module is required for testing, but it is not actually required to run the unit tests in tests/
• examples/test_*.py can not be run with Python (ipython is required). However, this is not mentioned anywhere.
• There only seems to be a small number of unit tests. The code coverage is very low.
• The eye diagrams in examples/test_dsp_core_functions.ipynb are not useful (just a blue box).
• Error when running ipython examples/test_clockRecovery.py: ValueError: operands could not be broadcast together with shapes (44000,) (43968,) (Line 121)
• Error when running ipython examples/test_fec.py: ModuleNotFoundError: No module named 'optic.modulation' (I think, it should be optic.comm.modulation)
• Also, the optic.fec module does not exist. I could not find any LDPC functionality in the documentation.
• Same issues of examples/test_fec.py also present in examples/test_fec.ipynb
• Error when running ipython examples/test_NLC_withDBP_WDM_transmission.py: ImportError: cannot import name 'phaseNoise' from 'optic.models.channels
• Large deviations between theory and Monte Carlo in examples/test_metrics.ipynb for small SNRs. What is the reason behind this? (I have also increased the number of samples by a factor of 10, which did not yield any improvement.)
• ImportError in examples/test_ofdm.py

Examples

• ImportError in examples/basic_EDFA.py

Paper (Minor Stuff)

• Acronym DSP is defined twice
• Non-breaking space ~ in "Fig.~3" is actually printed as a tilde.

[edsonportosilva]

Hello @lucydot @ebezzam. Sorry for my late reply. I am currently overloaded with the end of semester tasks. @ebezzam, thanks again for the time reviewing this project. Here is the reply to your queries.

@lucydot, @edsonportosilva below is my initial review. I realize that there is an examples folder, but is it expected to run everything or is there a notebook/script that gives an overview of the main features?

Functionality

• Installation: I would recommend putting the same installation in your README and in the documentation. You can create a README.rst file, and use that within your documentation (like this instead of creating a separate file so you don’t have to edit twice). I say this because I looked at the documentation for the installation steps; and python setup.py install didn’t work for me, but pip install . did (which was in your README).

1. Fixed. Now the documentation page should be updated as @ebezzam suggested. Moreover, we removed python setup.py install which is deprecated.

• Functionality: it would be useful to have a section or notebook to show the different features, as I’m not sure where I would start to try things out. I realize there’s an examples folder, but it is not clear where to start.

2. Fixed. We have added to the documentation page a getting started , as suggested by @lucydot, notebook where the reader can follow all the steps to set up a basic simulation and visualize performance results. This notebook also reproduces the plots shown in the JOSS paper. Another notebook was included with an advanced example, but I am still working to finish it.

• Performance: it would be interesting to quantify the speed-up thanks to GPU acceleration, as mentioned in Lines 61-64 of the paper.

3. We have added to the examples folder a notebook with a few basic benchmark results of CPU vs GPU processing time for simulations involving split-step Fourier algorithms, which are the most demanding processing tasks that could be performed in simulations . A mention of those results was included in the paper.

Documentation

• A statement of need: there is none in the documentation.

4. Fixed by 1.

• Automated tests: there are none nor manual steps to verify functionality.

5. Partially fixed. We have included a few automated tests. However, the scope of OptiCommPy (communications, optics, DSP) makes it difficult to set up meaningful unitary tests for all functions. For example, there are several algorithms of equalization and carrier recovery implemented, and each has particular requirements for correct operation, which depends on the specific scenario where they are tested. For those working in the field, this is well understood, though. Our first idea to workaround this issue was to add notebooks in the examples folder to work as a "systemic test" for the relevant functions, such that the user could see cases of use of a given function and then verify that the code was working properly. In practice, most of the code in OptiCommPy is validated with data from optical experiments, i.e. we have been able to use OptiCommPy to process data obtained from fiber optic transmission experiments and then verify that the code (e.g. physical models for the fiber, DSP algorithms, etc) work with real data. We could try to upload a few experimental traces to the repo and add a notebook to demonstrate real data processing, but the files to be uploaded may be large, and the experimental scenarios will not cover all functions. In any case, our plan is to include in the future as many automated tests as possible.

• Community guidelines: there is in the README, but as mentioned above for “Installation”, it could be nice to use the README in your documentation, as I didn’t see the contribution guidelines at first.

6. Fixed by 1.

Software paper

• A statement of need: The “Statement of need” starts great by giving the relevant context, but I feel like there’s a paragraph missing after Line 40 to explicitly say something like “OptiCommPy addresses this need by…” and who the target audience is (similar to what’s said in “Summary”).

7. Fixed

• Quality of writing: I think Lines 41 and onward should be in a separate section, e.g. with the title “OptiCommPy Code Structure”. And another section on what example scripts/code for the different features could be useful.

8. Fixed.

Miscellaneous

• Adding .ipynb_checkpoints and .pylint.d to your .gitignore

9. Fixed.

[edsonportosilva]

@taladjidi, thanks for your time reviewing this project. Please, see below the replies to the points you listed.

@lucydot @ebezzam

Sorry for the late reply, I was in leave. Below are my first comments.

Functionality

• Installation went without trouble, though I only tried through pip install .
• Examples: some of the notebooks do not run (while I did not test them all, basic_EDFA.ipynb seems to use an old package structure and does not run. I ran this first since I use an EDFA in the lab and it was the first example in the directory.

1. Fixed. The examples folder has been updated with some unused files removed and all the remaining notebooks should be running now without issues. Let me know if there is still something broken.

• Performance: I agree with @ebezzam that benchmarks curves would be a welcome addition. This could maybe be one of the tests ?

2. Fixed, partially. We have included a notebook in the examples folder with a benchmark for the most critical GPU processing tasks that may be required in the simulations available in the repository, but not covering the whole package. However, we are not claiming any particular performance gain when compared to any other simulator. We have just stated that GPU processing may speedup the simulations when compared to the code without it, and the results in the benchmark in the examples folder now provides evidence that supports this claim.

Furthermore, you will gain at least a factor of 2 performance on your FFT's if you explicitely define and cache the FFT plans (see get_fft_plan or directly through cupy.cuda.cufft) instead of using the cupyx.scipy.fft API.

• Performance / FFT: I would advise switching to a more performant FFT library than Numpy's since most of you DSP will rely on it. There are a lot of potential candidates, pyFFTW could be a drop in replacement. In the same vein, a lot of operations could be parallelized and JIT compiled using Numba.

3. Thanks for bringing up this point. Indeed, we have spent some time trying to optimize the performance of the split-step algorithms running with CuPy. I believe currently the implementation of FFT plans is done automatically by cupyx.scipy.fft. If you have any particular insights on how to further optimize performance, I would be glad to hear more.

Documentation

• Examples: Jupyter notebooks are a great way to introduce the various functionalities of your software. I found that the graphical outputs were very clear. Maybe it would be good to add Markdown blocks in between the code blocks to link the code with the physical models in use (how does one equation translate in terms of your package functionalities).

4. Thanks for the observation. Yes, that is our idea in the long run. However, what we observe is that this is not a big issue for the public that has an interest in using OptiCommPy, which is usually someone with a little bit of experience in the field. As you noted, the graphical outputs are sometimes more important for them. However, since we and other colleagues abroad are using OptiCommPy when teaching courses, having more information on the examples is more important.

• Physical models: I would expect the docstrings of the models to cite some relevant paper (when needed) explaining the model more in detail, or one or two latex equations to succintly define the used conventions etc ...

5. Fixed. We have included in the docstrings in most parts of the code citations to papers/books where one can find physical models, algorithms, equations, etc. described in detail.

• In the paper, you compare to commercial / proprietary solutions, maybe you could include a section of your Readme to explain what funcitonalities are supported, or some kind of migration guide ? In a similar fashion, maybe you could include some performance comparisons.

6. Indeed, that would be interesting. However, those commercial simulators are expensive and we do not have the licenses necessary to make a comparison with OptiCommPy. I didn't get the part of the "migration guide" that you have mentioned.

Software paper

• The manuscript overall is well written and has a very good introduction.
• It would be better to split the Statement of need and the functionalities that you introduce, in order to highlight the problem on one side and its solution on the other.

7. Fixed

• The package structure is maybe superfluous in the paper, and maybe could be replaced with benchmarks / comparisons with proprietary software (if you have access to it) ?

Overall it's a very nice project, while the examples need a bit of documentation work, they do seem to cover most of the functionalities of the package.

Thanks! :)

Best,

[edsonportosilva]

Hello @lucydot @klb2 thanks for your time reviewing this project. Please, see below the replies to the points you listed.

@lucydot @edsonportosilva

I have now completed my review. I tried to limit my comments to new ones that have not been covered in the reviews above.

In general, the package is interesting and contains a lot of features. The main issues are related to the tests and documentation, which lacks significant information (even missing whole modules).

Documentation

• It would be nice to add a simple example after the installation instructions to verify that the installation worked.

1 . Fixed, partially. The content in the getting started notebook available in the documentation should run without issues.

• In general, the documentation does not show any usage examples.

2. Fixed. We have added to the docs two examples, a basic and an advanced.

• Required version of matplotlib in documentation (>=1.4.3) does not match that in requirements/setup file (>=3.7.0)

3. Fixed

• Pandas is not stated as a dependency in documentation

4. Fixed

• It should be mentioned which scripts need to be run to reproduce the figures from the paper.

5. Fixed. The getting started notebook reproduces the plots from the paper.

• The documentation of the fec module is missing in the documentation.

6. Fixed. The FEC module has been removed because it was just unfinished work that has been discontinued.

• Community guidelines, e.g., a CONTRIBUTING.md file, are missing

7. Fixed. The documentation now provides guidelines for contributing to the code.

Tests

• tests/test_dsp.py does not run with the unittest package

8. Could you elaborate on that issue? I can see that all tests in the test folder run with pytest.

• The setup script (setup.py) says that the nose module is required for testing, but it is not actually required to run the unit tests in tests/

9. Fixed.

• examples/test_*.py can not be run with Python (ipython is required). However, this is not mentioned anywhere.

10. Fixed. The scripts '.py' in the examples folder where redundant copies of the notebooks made by jupytext and were removed to avoid confusion with actual .py tests files.

• There only seems to be a small number of unit tests. The code coverage is very low.

11. Yes, you are right. We are working towards increasing the coverage in the long run, but for reasons explained before this is not an easy task given the scope of the repository. However, the notebooks in the examples folder should be enough to verify the reproducibility and correctness for the main features of the repository.

• The eye diagrams in examples/test_dsp_core_functions.ipynb are not useful (just a blue box).

12. Fixed.

• Error when running ipython examples/test_clockRecovery.py: ValueError: operands could not be broadcast together with shapes (44000,) (43968,) (Line 121)

13. Redundant file removed.

• Error when running ipython examples/test_fec.py: ModuleNotFoundError: No module named 'optic.modulation' (I think, it should be optic.comm.modulation)

14. File removed.

• Also, the optic.fec module does not exist. I could not find any LDPC functionality in the documentation.

15. Fixed in 6.

• Same issues of examples/test_fec.py also present in examples/test_fec.ipynb

16. Fixed in 6.

• Error when running ipython examples/test_NLC_withDBP_WDM_transmission.py: ImportError: cannot import name 'phaseNoise' from 'optic.models.channels

17. Fixed.

• Large deviations between theory and Monte Carlo in examples/test_metrics.ipynb for small SNRs. What is the reason behind this? (I have also increased the number of samples by a factor of 10, which did not yield any improvement.)

18. Those results are correct because the performance comparison was between APSK and PSK, and since APSK tends to perform better than PSK, the curves make sense.

• ImportError in examples/test_ofdm.py

19. File removed.

Examples

• ImportError in examples/basic_EDFA.py

20. File removed.

Paper (Minor Stuff)

• Acronym DSP is defined twice

21. Fixed.

• Non-breaking space ~ in "Fig.~3" is actually printed as a tilde.

22. Fixed.

[lucydot]

Thank you very much @ebezzam @taladjidi @klb2 for your thorough and timely reviews 🌟

And excellent that you have addressed so many points in a short timeframe @edsonportosilva

@ebezzam @taladjidi @klb2 - please re-visit your checklist when you get a chance, and see if there are any outstanding issues blocking acceptance. Reading discussion, there may be some uncertainty around testing criteria - in which case it would be great to hear your thoughts and I can feed in with editorial viewpoint. The other key aspect was documentation.

[klb2]

@editorialbot generate pdf

[editorialbot]

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

[klb2]

Thank you for addressing my comments @edsonportosilva. I checked the code and documentation again and only some issues are remaining.

• The file Image("./figures/BasicIMDD.png", width=600) in the getting_started.ipynb notebook does not exist.

• I am still not sure about the correctness of the results in the test_metrics.ipynb notebook.

  • First, for low SNRs, the Monte Carlo simulation results systematically deviate from the theoretical results (Sections 1.1 and 1.2). This should not be the case, especially for small error rates with enough samples.
  • Next, in Section 2.1 of the notebook, the 256 QAM theory curve breaks the Shannon limit. This appears to be wrong, especially since the Monte Carlo simulations again show the expected behavior, e.g., see Fig. 1(a) in D. S. Millar, T. Fehenberger, T. Koike-Akino, K. Kojima and K. Parsons, "Coded Modulation for Next-Generation Optical Communications," 2018 Optical Fiber Communications Conference and Exposition (OFC), 2018.
    

• I did not see this before, but it should be "Fig. 2" instead of "Fig. 3" in the paper (line 82 on page 3).

Regarding point 8 from the previous comments: In the original submission, it was not clear/documented that pytest would be required to run the unit tests. Only the unittest module was imported in two out of the three files. But I saw that this is fixed now.

[edsonportosilva]

Hello @klb2, thanks again for the feedback,

Thank you for addressing my comments @edsonportosilva. I checked the code and documentation again and only some issues are remaining.

• The file Image("./figures/BasicIMDD.png", width=600) in the getting_started.ipynb notebook does not exist.

1. I don't know the problem in this case, because for me the figure is displayed both in the repository and the documentation page. Does anyone else see the same issue? @lucydot @ebezzam @taladjidi

• I am still not sure about the correctness of the results in the test_metrics.ipynb notebook.

  • First, for low SNRs, the Monte Carlo simulation results systematically deviate from the theoretical results (Sections 1.1 and 1.2). This should not be the case, especially for small error rates with enough samples.

2. The deviation from Monte Carlo to theoretical results is expected in this case for large QAM/PSK constellations. This happens because the expression used to calculate the error probability assumes that the error events are dominated by errors between neighboring QAM/PSK symbols, in a gray-mapped constellation. That is, every symbol error translates into a bit error. However, this is only true in moderate to low symbol-error-rate (SER) regimes. If the SER is above 1e-1, this assumption is not true anymore and the Monte Carlo results should deviate from the approximated theoretical curves, which would then underestimate the BER. I have added an explanation to the function's (theoryBER) docstring to clarify this point. Nevertheless, such regimes of high error rates are mostly of no practical relevance for standard digital communication systems, including fiber optic communications.

• Next, in Section 2.1 of the notebook, the 256 QAM theory curve breaks the Shannon limit. This appears to be wrong, especially since the Monte Carlo simulations again show the expected behavior, e.g., see Fig. 1(a) in D. S. Millar, T. Fehenberger, T. Koike-Akino, K. Kojima and K. Parsons, "Coded Modulation for Next-Generation Optical Communications," 2018 Optical Fiber Communications Conference and Exposition (OFC), 2018.
  

3. Thanks for reporting this issue. This deviation had not appeared before and I did a quick investigation to find that is related to the version of Numba. Results are fine with versions 0.54.0-0.57.0 and starting from version 0.58.0 this deviation appears. These results are obtained by integrating numerically the theoretical expression of the mutual information calculated between input and output for the memoryless Gaussian channel. It requires a double integral to be calculated with dblquad, whose argument is compiled with Numba to speed up the calculations. It turns out that different versions of Numba appear to be providing different precision for the calculation of this function, and some numerical instability is causing the results to deviate from the correct curves. Due to the lack of a better solution, for now, I have restricted the version of Numba in the requirements to 0.54.0-0.57.0 to work around this issue. See the results below.

• I did not see this before, but it should be "Fig. 2" instead of "Fig. 3" in the paper (line 82 on page 3).

4. Fixed.

Regarding point 8 from the previous comments: In the original submission, it was not clear/documented that pytest would be required to run the unit tests. Only the unittest module was imported in two out of the three files. But I saw that this is fixed now.

[klb2]

@editorialbot generate pdf

[lucydot]

@editorialbot set v0.9.0-alpha as version

[editorialbot]

Done! version is now v0.9.0-alpha

[lucydot]

@editorialbot check references

[editorialbot]

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1109/50.622902 is OK
- 10.1109/JSTQE.2010.2044751 is OK
- 10.1109/JLT.2009.2039464 is OK
- 10.1109/JLT.2020.2996188 is OK
- 10.1109/JLT.2017.2786351 is OK
- 10.1109/JLT.2017.2662082 is OK

MISSING DOIs

- No DOI given, and none found for title: Fiber-Optic Communication Systems
- No DOI given, and none found for title: Digital Communications
- No DOI given, and none found for title: Robochameleon: A matlab coding framework and compo...
- No DOI given, and none found for title: OptiSystem
- No DOI given, and none found for title: VPItransmissionMaker™ Optical Systems 
- No DOI given, and none found for title: Synopsys OptSim for Optical Communication
- No DOI given, and none found for title: Optilux, the optical simulatr toolbox.
- No DOI given, and none found for title: CuPy: A NumPy-Compatible Library for NVIDIA GPU Ca...
- No DOI given, and none found for title: CUDA, release: 10.2.89

INVALID DOIs

- None

[lucydot]

Hi @edsonportosilva, can you check and confirm that there are no DOIs available for the references above? I can see they are either software or book references, so less likely to be available than for journal references.

After that I will recommend acceptance to the track editors 🥳

[edsonportosilva]

Hi @lucydot,

Hi @edsonportosilva, can you check and confirm that there are no DOIs available for the references above? I can see they are either software or book references, so less likely to be available than for journal references.

I have checked and there is no DOI available for those references, mostly because they are either software or physical books, as you noted.

After that I will recommend acceptance to the track editors 🥳

Great to hear that and thank you for your guidance during this peer review process 😊

[lucydot]

@editorialbot recommend-accept

[editorialbot]

Attempting dry run of processing paper acceptance...

[editorialbot]

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1109/50.622902 is OK
- 10.1109/JSTQE.2010.2044751 is OK
- 10.1109/JLT.2009.2039464 is OK
- 10.1109/JLT.2020.2996188 is OK
- 10.1109/JLT.2017.2786351 is OK
- 10.1109/JLT.2017.2662082 is OK

MISSING DOIs

- No DOI given, and none found for title: Fiber-Optic Communication Systems
- No DOI given, and none found for title: Digital Communications
- No DOI given, and none found for title: Robochameleon: A matlab coding framework and compo...
- No DOI given, and none found for title: OptiSystem
- No DOI given, and none found for title: VPItransmissionMaker™ Optical Systems 
- No DOI given, and none found for title: Synopsys OptSim for Optical Communication
- No DOI given, and none found for title: Optilux, the optical simulatr toolbox.
- No DOI given, and none found for title: CuPy: A NumPy-Compatible Library for NVIDIA GPU Ca...
- No DOI given, and none found for title: CUDA, release: 10.2.89

INVALID DOIs

- None

[editorialbot]

⚠️ Error preparing paper acceptance. The generated XML metadata file is invalid.

Element isbn: [facet 'minLength'] The value has a length of '9'; this underruns the allowed minimum length of '10'.
Element isbn: [facet 'minLength'] The value has a length of '9'; this underruns the allowed minimum length of '10'.
Element isbn: [facet 'minLength'] The value has a length of '9'; this underruns the allowed minimum length of '10'.

[editorialbot]

I'm sorry @edsonportosilva, I'm afraid I can't do that. That's something only editors are allowed to do.

[edsonportosilva]

Hello @lucydot, I guess something was wrong with the ISBN information in the .bib file added to the paper. I have removed this information from the file since it is not displayed in the paper. Now, perhaps the process will run without errors.

[lucydot]

Thanks @edsonportosilva, I'll try again now.

[lucydot]

@editorialbot recommend-accept

[editorialbot]

Attempting dry run of processing paper acceptance...

[editorialbot]

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1109/50.622902 is OK
- 10.1109/JSTQE.2010.2044751 is OK
- 10.1109/JLT.2009.2039464 is OK
- 10.1109/JLT.2020.2996188 is OK
- 10.1109/JLT.2017.2786351 is OK
- 10.1109/JLT.2017.2662082 is OK

MISSING DOIs

- No DOI given, and none found for title: Fiber-Optic Communication Systems
- No DOI given, and none found for title: Digital Communications
- No DOI given, and none found for title: Robochameleon: A matlab coding framework and compo...
- No DOI given, and none found for title: OptiSystem
- No DOI given, and none found for title: VPItransmissionMaker™ Optical Systems 
- No DOI given, and none found for title: Synopsys OptSim for Optical Communication
- No DOI given, and none found for title: Optilux, the optical simulatr toolbox.
- No DOI given, and none found for title: CuPy: A NumPy-Compatible Library for NVIDIA GPU Ca...
- No DOI given, and none found for title: CUDA, release: 10.2.89

INVALID DOIs

- None

[editorialbot]

👋 @openjournals/pe-eics, this paper is ready to be accepted and published.

Check final proof 👉📄 Download article

If the paper PDF and the deposit XML files look good in openjournals/joss-papers#5479, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

[kyleniemeyer]

@editorialbot accept

[editorialbot]

Doing it live! Attempting automated processing of paper acceptance...

[editorialbot]

Ensure proper citation by uploading a plain text CITATION.cff file to the default branch of your repository.

If using GitHub, a Cite this repository menu will appear in the About section, containing both APA and BibTeX formats. When exported to Zotero using a browser plugin, Zotero will automatically create an entry using the information contained in the .cff file.

You can copy the contents for your CITATION.cff file here:

CITATION.cff

cff-version: "1.2.0"
authors:
- family-names: Silva
  given-names: Edson Porto
  name-particle: da
  orcid: "https://orcid.org/0000-0003-4230-9121"
- family-names: Herbster
  given-names: Adolfo Fernandes
  orcid: "https://orcid.org/0000-0001-6194-1160"
doi: 10.5281/zenodo.11450597
message: If you use this software, please cite our article in the
  Journal of Open Source Software.
preferred-citation:
  authors:
  - family-names: Silva
    given-names: Edson Porto
    name-particle: da
    orcid: "https://orcid.org/0000-0003-4230-9121"
  - family-names: Herbster
    given-names: Adolfo Fernandes
    orcid: "https://orcid.org/0000-0001-6194-1160"
  date-published: 2024-06-11
  doi: 10.21105/joss.06600
  issn: 2475-9066
  issue: 98
  journal: Journal of Open Source Software
  publisher:
    name: Open Journals
  start: 6600
  title: "OptiCommPy: Open-source Simulation of Fiber Optic
    Communications with Python"
  type: article
  url: "https://joss.theoj.org/papers/10.21105/joss.06600"
  volume: 9
title: "OptiCommPy: Open-source Simulation of Fiber Optic Communications
  with Python"

If the repository is not hosted on GitHub, a .cff file can still be uploaded to set your preferred citation. Users will be able to manually copy and paste the citation.

Find more information on .cff files here and here.

[editorialbot]

🐘🐘🐘 👉 Toot for this paper 👈 🐘🐘🐘

[editorialbot]

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

0. Check final PDF and Crossref metadata that was deposited 👉 Creating pull request for 10.21105.joss.06600 joss-papers#5483
1. Wait five minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.06600
2. If everything looks good, then close this review issue.
3. Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? Notify your editorial technical team...

[kyleniemeyer]

Congratulations @edsonportosilva on your article's publication in JOSS! rePlease consider signing up as a reviewer if you haven't already.

Many thanks to @klb2, @ebezzam, and @taladjidi for reviewing this, and @lucydot for editing.

[editorialbot]

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](https://joss.theoj.org/papers/10.21105/joss.06600/status.svg)](https://doi.org/10.21105/joss.06600)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.06600">
  <img src="https://joss.theoj.org/papers/10.21105/joss.06600/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: https://joss.theoj.org/papers/10.21105/joss.06600/status.svg
   :target: https://doi.org/10.21105/joss.06600

This is how it will look in your documentation:

We need your help!

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

• Volunteering to review for us sometime in the future. You can add your name to the reviewer list here: https://reviewers.joss.theoj.org/join
• Making a small donation to support our running costs here: https://numfocus.org/donate-to-joss