Make handling of t consistent

[hagenw]

The time t is handled in some functions in samples and in others in seconds. To avoid confusions like the one of #108, we should make this more consistent. My proposal would be to switch completely to t in seconds as we also do in the Python version.

So, let's start with a list of all the functions that uses t in samples which should be reviewed if we easily could change them:

• [x] delayline()
• [x] warning_if_zero()
• [x] greens_function_imp()
• [x] sound_field_imp_line_source()
• [x] sound_field_imp_localwfs()
• [x] sound_field_imp()
• [x] sound_field_imp_nfchoa()
• [x] sound_field_imp_plane_wave()
• [x] sound_field_imp_point_source()
• [x] sound_field_imp_wfs()
• [x] time_response_wfs()
• [x] time_response_localwfs()
• [x] time_response_nfchoa()

In addition we have a few functions that use samples, but in those cases it is not directly referring to the time t, but to something like filter length. For those I would propose to stay with samples, but we should also review them:

• [x] dummy_irs()
• [x] general_least_squares()
• [x] hann_window()

As a first step it would be great to check, if I have missed some function in the listing.

[hagenw]

I have no updated all functions I was able to identify that used t in samples to use t in seconds instead. for the user the biggest change will be in the sound_field_imp related functions as there t must be provided. What will be much easier now is the incorporation of the returned delay_offset values as they now always are in seconds and can directly be applied.

The three functions I listed above that still use samples instead of seconds should stay with this in my opinion.

@fietew: could you please have a look at my changes.

[fietew]

Just a short note about the handling of this changes: As @hagenw already mentioned, this pull request will have quite an impact on the behavior of the time-domain functionalities. I would suggest to create one minor release (2.2.x) right before and one right after merging this pull request. This enables the correct referencing of the used implementation, e.g. in scientific publications.

[hagenw]

OK, I agree, we should first make a release, then integrate and then make another release.

[fietew]

One additional thing: Under MATLAB, I had problems to execute test_plot(true):

Attempt to execute SCRIPT test_plot as a function:
/home/winter/projects/sfstoolbox/test_plot.m

trying test_plot without argument got me this single plot

After I switched to the validation directory, everything worked as expected. If think, there is some kind of naming issue. Can somebody else test the function under MATLAB?

[hagenw]

@chris-hld could you please verify if you can reproduce the test_plot problem under Matlab. Maybe we should simply rename that function.

[fietew]

Maybe we should simply rename that function.

I agree.

[chris-hld]

No, I cannot recreate that problem. Executing: SFS_start; test_plot() , results in correct Not enough input arguments.. Is there maybe another test_plot() in your path?

[fietew]

Let's just rename the function in order to finally merge this pull request.

[hagenw]

I renamed the function to test_plotting(). Unfortunately we are not ready for merging as the documentation (files under doc/) has to be adjusted. I will have a look at it.

[hagenw]

I have updated the documentation.

@chris-hld could you please run generate_plots and push only the following files afterwards:

sound_field_imp_nfchoa_25d.png
sound_field_imp_nfchoa_25d_dB.png
sound_field_imp_multiple_sources_dB.png

@fietew could you please run the test_all a last time to see if still everything is working.

After that we should be ready to merge.

[fietew]

test_all finished without significant errors (besides to_be_implemented).

Looking at the plots: test_colormaps: In the time-domain plots, there is no soundfield visible.

[hagenw]

Thanks, there was indeed one time given in samples left. I have fixed test_colormaps() now.

[chris-hld]

That should be it

[hagenw]

I have cleaned up the history of this and included it into master as seven different commits.