JOSS review

[james-trayford]

Some of the technical issues arrising from my review for JOSS journal (openjournals/joss-reviews#5819)

Suggested direct code changes pertaining to these can be found in Pull Request #10. These are code changes that seemed small or simple enough for me as a newcomer to the sonoUno source code, though some comments below refer to more involved changes.

To reiterate what I said in the review thread, I enjoyed exploring sonoUno and it's functionality, and think it represents a valuable tool in providing accessible sonification of 1D data (in the pitch-mapping tradition) for research applications. I congratulate the developers on their work and hope that the comments below and attached pull request are constructive towards the future development of the code.

Functionality Issues & Comments

• The Continuous sound option is not continuous in the GUI, becoming more staccato as tempo increased. This seems to be related to the Performance section below (performance-limited playback)
• I think an option to re-load marks would be useful. Generally I'm not sure why saving should delete existing marks. I think people may wish to keep these and can choose to clear all marks after saving if not. Just stopping the save option deleting all marks in a session would be an improvement I think.
• Saved sound different from GUI sound. For this reason, perhaps it is better to refer to "Render to Sound File" or similar rather than "Save sound", which I think implies that the sound experienced in the GUI and output should be the same.
  • Tempo often very different (see performance limit discussion below)
  • GUI varies abcissa linearly in time, while saved sound spaces abcissa (x) data points evenly by their tabulated order. for abcissa value to be meaningful, i would expect linearly varying in x to be the correct functionality (such that uneven x-spacing can be heard rhythmically). This was tested using files with a log-spaced abcissa added to the test data in Pull Request JOSS review suggestions #10.
  • inputting a data file with a non-monotonically increasing first column (abcissa value), leads to a scrambled order of playing. My intuition is that either values should be sorted by abcissa value so that they play in order, or there should be some error / warning that user has input a file with a non-monotonically increasing first column
• The two large classes in sonoUno.data_import are identical apart from their names and __init__ docstrings. Can one be removed? if these are placeholders for two classes that will diverge later, could consider using class inheritance to minimise repeated code and make the source code more readable.
• Strange behaviour swapping x and y axis in some cases, see e.g. sinusoid, cursor eventually starts scrolling infinitely to the right. Can prevent this happening or fix? This could perhaps be fixed by the re-ordering of swapped axes, mentioned next
• On swapping axes, I think the playback of data points should be re-ordered by the new abcissa, otherwise we see incorrect behaviour. For default decrease.txt example, we hear a decreasing pitch as expected. When swapping axes however, hear an increasing pitch which is incorrect (y decreases with x as x decreases with y) - this is also because the playback order of data points just goes by the row order rather than actual abcissa value.
• Why put the Column0,...ColumnN strings in that data frame in sonoUno/data_import/data_import.py, rather than using them as column names? could also generate these strings when needed from column indexes alone. mixing numeric and string entries could lead to confusion down the line I think.
• Data files containing inf's and nan's in abcissa or ordinate values load but behave in broken ways (e.g. all sound same pitch). Perhaps better to throw an error when loading these, or filter out non-finite values? Could use e.g. numpy.isfinite function.
• Think the 'Any' marker should be renamed to 'None'
• The shortest iteration fed to the wx timer is 10 ms, aiming to play a sound every 10s. this frequency is 100 Hz so can interfere with audible frequencies. This is why on exporting a sound like the galaxy spectrum example at full tempo a constant 100Hz artefact sound can be heard dominating the audio (though minimised by judicious ADSR enveloping). I would suggest changing the max tempo to 50 ms i.e. 20 Hz (low limit of human hearing), which should avoid audible artefacts. This also brings the tempo of the saved audio close to the GUI generated audio, so they are more representative of each other (see performance section below).
• The envelope sound option is a sonified pitch graph of the ADSR envelope. However I think it would be more natural and efficient to just play back a note with that envelope to allow users to quickly find a setting they like if they cannot access the visual graph - in this case I think the exact numerical shape of the ADSR envelope are less important than how it sounds subjectively for the user.

Performance

I thought it would be worth also adding some thoughts about code performance that could help the code. Something found while experimenting with the code is that the speed difference between GUI playback and exported audio appears to be because GUI playback is limited by the CPU performance of the user. This can be seen for example that at max tempo GUI player is significantly faster when the graphing is turned off ("Data Display" unchecked). This can be seen in the below performance graph, made by adding timing statements to print the interval between each _sonificationloop_event - we see that with graphing on the playback is typically ~ 200 ms while of it is closer to 100 ms.

It may be worth considering a max tempo that can reasonably be achieved by a user's computer in the GUI (e.g. ~200ms in my case a 2020 macbook pro) ,so that saved audio and GUI playback audio are more similar. This would also mean that the note spacings are more regular as when pushed to the tempo limit, note spacings can vary by large factors (just due to how long the computer take to process the loop step at a given time).

For larger data sets it seems it may be desirable to have faster iteration through the data points (e.g. to ~50 ms as mentioned above - this would reduce reproduction time of the full example SDSS spectrum from ~12 m 50 s in GUI to ~3m 12 s) I think there may be ways to improve performance further in the _sonificationloop_event to speed things up (e.g. checking NaNs ahead of time, etc.) that may be worth looking into in future.

[johicasado]

Thank you very much @james-trayford for the detailed explanation and all the time dedicated, I appreciate it a lot!

I will put all of these into the agenda, all of the suggestions are very important and some of them also appear in the user testing.

Only some minor comments just in case, about the sort option I think it is great! I don't remember at this moment, which type of data needs to maintain the x value unsorted, but if that problem arises we can just put a function to undo or something like that. About the "I would expect linearly varying in x to be the correct functionality" I will check that. The column names must be included just in case the file doesn't provide one, but maybe it is working badly right now. I will check it.

About the inf's and NaN's values, they are important in some cases, I don't know the better way to proceed with that, my first intuition was to do a survey with the possible option, and evaluate what people need in that cases; about the soft capabilities, I'm thinking using tickmarks to differentiate them from cero, the warning message that you suggest, some section into the configurations; this only some first thoughts.

The GUI requirements and sound production gave me headaches from the beginning. We talked about it in the Audible Universe meeting I think. One option that probably works is to allow the user to deactivate the graph and sound synchronization.

I will work on this topic and post here if I have some news! or I'll open a new issue for a specific exchange.
Thank you very much!

[james-trayford]

Great, thanks for the explanations! Yeah, some of these questions seem more complicated than others, and I appreciate not all the suggestions may be feasible. In case it wasn't clear, I don't consider addressing all these issues as necessary for the actual JOSS publication, with those specific points addressed in the review thread comment: openjournals/joss-reviews#5819 (comment)