sfstoolbox/sfs-python

Rename frequency domain and time domain submodules

mgeier opened this issue · 31 comments

Currently they are called mono and time, which is inconsistent.

Possible options:

  • frequency + time
  • fd + td
  • f + t

Another option, which is currently used in the equation references on http://sfstoolbox.org/, is to use capital letters to denote the frequency domain.

Any other ideas?

Additionally, there are some functions, that are categorized in either, but are actually valid for both domains.
E.g. sfs.mono.drivingfunction.source_selection_... comes to my mind.

spors commented

I would vote for fd for frequency-domain and td for time-domain functions. However at some point we might also have to distinguish between temporal and spatial frequency domain. For instance when implementing the spectral divison method (SDM).

at some point we might also have to distinguish between temporal and spatial frequency domain

Maybe we could consistently use "modal domain" for the various spatial frequency domains.

I'm also in favor of "time domain", "frequency domain", and "modal domain", but I'm not completely sure if it is sufficient to use td, fd, md, or if those names should be more self-explanatory to the user.

There are two things I like about the current (time, mono) solution:
1.) They are short and have equal length.
2.) You can pronounce them.

As @spors has pointed out frequency would have a problem. It might be that the usage of time and mono is not completely consistent, but they are not as problematic.
modal as a third option would be fine with me.

I do not see a problem with using freq for the time-frequency domain, since we do not have the (one) space-frequency. Most likely, we will refer to the specific geometry-dependent domains cht, sht, kx or pwd, instead. Right now, I do not see any functionality we can implement without knowing the specific space-frequency domain. Hence, modal is too general, imho.

We should also consider, that modal (or the specific domain) has to be combined with both, time and freq. I think, that an additional level of hierarchy e.g. time.cht and freq.cht might be better suited for that.

It would be very helpful if we could agree on one (at least temporary) solution until end of February as I will adjust the links to the functions in the theory section accordingly. Alternatively, I could stay with what I have there at the moment and we will change it later when this is solved.

I'm especially interested in the question if I should go with:

  1. time, mono
  2. time, frequency
  3. time, freq
  4. td, fd
  5. t, f

I'm especially interested in the question if I should go with:

  1. time, mono
  2. time, frequency
  3. time, freq
  4. td, fd
  5. t, f

In order of preference: 3, 4, 5, 1, 2

We should also consider, that modal (or the specific domain) has to be combined with both, time and freq. I think, that an additional level of hierarchy e.g. time.cht and freq.cht might be better suited for that.

I agree with @fietew.

I also prefer 3.

Thanks for the input, 3. (time and freq) seams also reasonable to me.

fs446 commented

My choice in order: 3, 4, 5, 2. Don't like 1.

Go ahead for time and freq. The modal issues should be checked after this change within the driving function files.

spors commented

I would vote for 3, 4 and not for the other variants.

So it looks like freq/time is the favorite, followed by fd/td.

I don't really like the name freq, because it feels like an unnatural abbreviation and it is very hard to pronounce. I would argue that it is not Pythonic to have such abbreviations, but we should rather have either acronyms (like sfs) or full names (like tapering). But I have to contradict myself immediately, because we also have sfs.util, which is the same kind of abbreviation (but probably easier to pronounce). OTOH, this is really the only exception we currently have.

The second problem that I have with both freq and time is that (even if you imagine the full name frequency) they are still abbreviations at another level, because they actually stand for "frequency domain" and "time domain". The words "frequency" and "time" on their own don't contain the necessary information that's needed for the situation we are using them in.

The choice fd/td has none of these problems.
But it has another problem, it's a bit hard to see what those letters mean.

I think we should look at it in context. @hagenw has already shown in #119 how freq/time would look like. I've just created #135 to show how fd/td would look like.
I've also created a variation #136, which is based on my suggestion #130.

I'm still unsure which of the options is really the best. Probably someone can come up with an even better option that we haven't discussed before?

Please look at the different suggestions, look at how the example scripts would look like, and create the Sphinx docs and look how those would look like.

If everyone still prefers freq/time, we can use that, but I just wanted to give you the opportunity to look at it in context, especially since the whole module structure has changed quite significantly since we started this discussion.

fs446 commented

My choice in order: 3, 4, 5, 2. Don't like 1.

Go ahead for time and freq. The modal issues should be checked after this change within the driving function files.

My initial favorite choice changed with the new structure. Now I can perfectly follow the argumentation of fd/td, my second choice. It looks very nice in the API and is of all solutions the most consistent, so I'd vote to take this road.
For me, introducing the fancy part is a cool add-on, however I wouldn't mind not to have this for the td/fd, since the naming is short enough by itself.

I think freq is not hard to pronounce and freq/time look more intuitive than fd/td. In addition, I think the d is redundant information. I would still vote for freq/time.

Since we have the possibility of using the fancy import #130,
what about just naming time/frequency?

I think freq is not hard to pronounce

freak

I think I would prefer frequency over freq because of my first point above (#25 (comment)).
But it would still not help with my second point ...

Based on the availability of #130 I also thought about using other long names, e.g. monochromatic/broadband, but I'm not really liking it. But I guess it would just be a matter of getting used to it.

@hagenw

In addition, I think the d is redundant information.

It seems non-redundant to me, but probably you have another view.
How would you interpret something like sfs.time.source.point()?
Something for timing point sources?

I've created another variation using monochromatic/broadband: #141.

This kinda requires #130, otherwise it can be annoyingly verbose.

This may not sound convincing when reading about it here, but you should check out the code examples and the generated Sphinx docs, it actually looks quite good IMHO!

fs446 commented

Well, a better counterpart of monochromatic would be polychromatic,
which only makes me like time / frequency even more.

We didn't have an example for frequency/time yet, now we have it: #144.

fs446 commented

Nice, thank you very much!
My priority list (highest to lowest)

  1. td/fd (short, most consistent with other abbreviations we already use, sfs, wfs etc.) , can include fancy import
  2. time/frequency, include fancy import should be a must have

I don't like
3. time/freq
4. broadband/monchromatic
due to discussed flaws and inconsistencies.

I would be fine with both 1. and 2.

  1. Also has the advantage that equation links in the documentation would be short
  2. It's still a little bit more obvious, but @mgeier is most probably right that sfs.time.source.point() cannot be interpreted by new users that easily.

I still like time/frequency slightly more than time/freq or td/fd.

I want to draw your attention to the following point.

If time/frequency is used, the spatial frequency should be also spelled out. Since those can be either in the time- or frequency domain (as @fietew mentioned in #25 (comment)), the combination would be like

sfs.time.spherical_harmonics.wfs.point()
sfs.frequency.circular_harmonics.nfchoa.plane()

which are too long.

If we use td/fd, the spatial frequency domain should be something like shd and chd (for spherical harmonics domain and circular harmonics domain).

sfs.td.shd.wfs.point()
sfs.fd.chd.source.plane()
sfs.fd.kx.sdm.plane().

Maybe too many abbreviations?
IMHO, they should be more self-explanatory.

fs446 commented

Good point, @narahahn!

sfs.td.shd.wfs.point()
sfs.fd.chd.source.plane()
sfs.fd.kx.sdm.plane().
assumes that we handle spatial frequency modules as well then. We should make sure that this is our intention. I have no idea about it yet.

td/fd and time/frequency are both fine for me.

@narahahn

What about using these two right now:

sfs.fd
sfs.td

... and introducing those later, when needed:

sfs.td_shd
sfs.fd_chd
sfs.fd_kx

I don't think it's necessary to introduce a whole other level of submodules.
And if we would do that, we would need additional names for the current modules:

sfs.td.???.wfs.point()
sfs.fd.???.source.plane()

If you do know appropriate names, we could change the current module names to

sfs.fd_???
sfs.td_???
fs446 commented

What about using these two right now:

sfs.fd
sfs.td

... and introducing those later, when needed:

sfs.td_shd
sfs.fd_chd
sfs.fd_kx

I'd vote for this. I am slightly against submodules, however I currently have no idea, what driving functions/filters we have to expect within the different spatial domains. For those I can imagine of the cited proposal is perfectly fine for me.

I don't think it's necessary to introduce a whole other level of submodules.

Yes, you're right. It doesn't make much sense to have those submodules.
At the end, the driving functions need to be transformed into the space-domain anyway.
Unlike time and frequency domain, it is unlikely to work exclusively in one domain,
either space or spatial frequency domain.

This should be a topic for future discussion.

What about using these two right now:

sfs.fd
sfs.td

My final vote for both time/frequency and td/fd.

I looks like #135 is the winner.