JOSS review

[e-dub]

Very nice work here. After my initial interaction with the software and paper, I have made the following remarks

General

• [x] heavy git pull relative code size, nearly 200 MB for 5 MB of code. Is it pulling some old data in the .git folder? Maybe this can be reduced?
• [x] pip install slightly different name than code. Can this be unified? pyoma2 vs pyoma-2
• [ ] Possible to provide non-ipython examples?

Use

General

• [x] Can expected (theoretical) results be added to show correctness of results?

Example1.ipynb:

• [x] Can be explained what we are looking at? Is this some kind of synthetic data or real measurements or the system parameters?
• [x] Maybe plot the data at beginning to bring it to life and understand what we are doing?
• [x] Please explain a bit what is going on, what is expected and why
• [ ] Cell 1 is the same problem as rest of example?
• [x] maybe add "(downsampling)" to decimate. q=30 meaning every 30th sample?
• [x] analise -> analyze
• [x] instanciate -> instantiate, or initialize
• [x] Cell 4 gives several outputs with red background. Is this desired behavior?

Example2.ipynb:

• [x] include quick description of problem beyond provided reference may help new users (and this reviewer ;-)
• [x] several red messages when running.
• [ ] error stating pyvista not installed despite being installed. Using Ubuntu 24.10, tested with 3.11 and 3.12. Cell 20 properly displays figure.

Example3.ipynb:

• [ ] Same pyvista error as above for cells 7 and 9

Example4.ipynb:

• [ ] Same pyvista error as above for cell 5

Paper:

• [x] reference to original module pyOMA, i.e. 10.1016/j.softx.2022.101216
• [x] Expand to touch on deltas to pyOMA? Add need for pyOMA2 w.r.t. pyOMA?
• [x] Consider motivating work with possible uses and how this software meets gaps?
• [x] algorithm list as bulleted list
• [x] "the following algorithmS"
• [x] "key references" are listed without explaining why they are referenced. I recommend expanding this part.
• [ ] users in community for pyOMA or pyOMA2? I recommend expanding to mention how used.
• [x] Number list for module primary levels?
• [x] asterisks needed in lines 48ff?
• [x] After reading, I do not fully understand what this module does and what input and output is.

[dfm88]

General

• [ ] heavy git pull relative code size, nearly 200 MB for 5 MB of code. Is it pulling some old data in the .git folder? Maybe this can be reduced?[ ] pip install slightly different name than code. Can this be unified? pyoma2 vs pyoma-2[ ] Possible to provide non-ipython examples?

Dear @e-dub,

thank you for your time in reviewing this and to pointing out the issue. You are absolutely correct , the git history contains some large files that were committed and later removed in the past, leading to the current situation where cloning the repository downloads way more data than necessary.

@dagghe we should discuss whether to:

1. Remove these old large files from the history (this would require rewriting the entire git history)
2. Or alternatively, add a note in our contributing guidelines recommending contributors to use a shallow clone if they don't need the full history, so they wouldn't download the old huge files, with

git clone --depth=20 https://github.com/dagghe/pyOMA2.git

Please let us know if you have any preference between these two approaches.

Best regards

[dagghe]

Hi @e-dub, thanks for the time spent reviewing our module! We will address your comments in the coming days.

[faroit]

cross referencing the review issue here: https://github.com/openjournals/joss-reviews/issues/7656

[dfm88]

General

[x] pip install slightly different name than code. Can this be unified? pyoma2 vs pyoma-2

Unfortunately it is not possibile the name pyOMA2 which was used for the very first release was wrongly deleted and PyPi does not allow to reuse the same name even if the repo was deleted, so we adopted pyOMA-2

[faroit]

@dagghe @dfm88 is there some update on when the issues are addressed?

[faroit]

@dfm88 @dagghe can you update us here or tell us if you need more, please?

[dagghe]

@faroit sorry I have been very busy with work!! I´ll do my best to address the issues during the easter holidays

[faroit]

@dagghe thanks for the update.

[dagghe]

Use

General

[ ]  Can expected (theoretical) results be added to show correctness of results?

The exact result of the numerical simulations included in the examples are already reported in the files.

Example1.ipynb:

[ ]  Can be explained what we are looking at? Is this some kind of synthetic data or real measurements or the system parameters?

The description of the example at hand has been reformulated and expanded.

[ ]  Maybe plot the data at beginning to bring it to life and understand what we are doing?

A plot of the data is already shown for example 2, which is a real dataset and thus shows a bit more interesting time histories. Here they the time histories would be white noises thus not so interesting/informative.

[ ]  Please explain a bit what is going on, what is expected and why

The description of the example at hand has been reformulated and expanded.

[ ]  Cell 1 is the same problem as rest of example?

I am not sure if I understand the question. In cell 1 we import the necessary modules and the example data (with the known modal parameters, since it is a numerical example).

[ ]  maybe add "(downsampling)" to decimate. q=30 meaning every 30th sample?

Even though the two terms are often used interchangeably there is a subtle difference: in downsampling one only reduce the sampling rate of a signal, while with decimation one first apply a low-pass filter to avoid aliasing and then downsample the filtered signal.

[ ]  Cell 4 gives several outputs with red background.  Is this desired behavior?

This is the expected behaviour, the outputs inform the users of what is going on and the progress of the analysis.

Example2.ipynb:

[ ]  include quick description of problem beyond provided reference may help new users (and this reviewer ;-)

The description of the problem at hand has been reformulated and expanded.

[ ]  several red messages when running.

As explained above, this is the expected behaviour, the outputs inform the users of what is going on and the progress of the analysis.

[ ]  error stating pyvista not installed despite being installed.  Using Ubuntu 24.10, tested with 3.11 and 3.12. Cell 20 properly displays figure.

Example3.ipynb:

* [ ]  Same pyvista error as above for cells 7 and 9

Example4.ipynb:

* [ ]  Same pyvista error as above for cell 5

Could you give us a feedback if you still get this error message?

Paper:

[ ]  reference to original module pyOMA, i.e. 10.1016/j.softx.2022.101216

We have included a reference to the old module.

[ ]  Expand to touch on deltas to pyOMA? Add need for pyOMA2 w.r.t. pyOMA?

We have modified the text, hopefully making more clear the improvements wrt to pyOMA.

[ ]  Consider motivating work with possible uses and how this software meets gaps?

Hopefully the modification made to the paper better explain these aspects.

[ ]  algorithm list as bulleted list
[ ]  "the following algorithmS"

Corrected

[ ]  "key references" are listed without explaining why they are referenced. I recommend expanding this part.

The section has been revised.

[ ]  users in community for pyOMA or pyOMA2? I recommend expanding to mention how used.

I am not sure I understand the question/comment.

[ ]  asterisks needed in lines 48ff?

Fixed.

[ ] After reading, I do not fully understand what this module does and what input and output is.

I am sorry you feel this way; the software is highly technical and aimed at an audience primarily consisting of engineers working on in situ dynamic analysis and structural health monitoring. The input are the acceleration time histories acquired from a structure and the output are the modal properties of the structure under investigation.

[dagghe]

Dear @e-dub, @faroit, again sorry for the delay, hopefully the comments have now been fixed with the last commit!

[faroit]

@e-dub can you please check the changes and close this issue if valid?

[faroit]

@e-dub can you please check the changes and tell us if this addresses your original comments?

[e-dub]

@e-dub can you please check the changes and tell us if this addresses your original comments?

Please see checklist above. Example5 and Extra1 are showing errors when I run them and added new points accordingly.

[dagghe]

Dear @e-dub check the new release.

[faroit]

@e-dub can you check if the issues are addressed sufficiently, please?