These are chat archives for SHTOOLS/SHTOOLS

7th
Nov 2016
MMesch
@MMesch
Nov 07 2016 08:58
yea symlog is nicer!
I even thought about normalizing the m's to 1
it makes even more sense because it is a relative orientation
this means using m/l instead of m for the y-axis
(we can add this as an option)
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 12:09
Let me know if you want me to add unit=l2norm. This is trivial if you think its useful : SHTOOLS/SHTOOLS#65
MMesch
@MMesch
Nov 07 2016 12:55
yes!
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:04
       'l2norm'
        returns the sum of the magnitude squared of the coefficients for all
        angular orders as a function of spherical harmonic degree l.
is that ok?
MMesch
@MMesch
Nov 07 2016 13:15
yes.
I think that gives enough options for the moment
it is easy to add any other afterwards
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:17
I don't think that there is a point adding this option to the SHWindow methods, given that the coefficients are forced to be 4pi normalized, and the power spectrum=l2norm. Is that ok with you?
MMesch
@MMesch
Nov 07 2016 13:20
yes, agree
if so, we should just provide a method that returns a window as a SHCoeff.
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:20
ok. its pushed, but it is tacked on to the end of the rename PR.
We have that, and now it is called SHWindow.return_coeffs (and return_grid). These are the only two methods that I'm not happy with the name, so let me know if you have anything better.
MMesch
@MMesch
Nov 07 2016 13:22
no :)
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:22
The only other options I see are return_shcoeffs or return_SHCoeffs
MMesch
@MMesch
Nov 07 2016 13:22
not right now, haha
hm yes that's maybe more clear. However it might be good to avoid using the actual class name
Mark, can we change all function names to have underscores? At some point this has to be done
the cross power spectrum can also become cross spectrum
saves some space and is equally as clear imho
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:26
can you propose the changes here: SHTOOLS/SHTOOLS#69 ?
If I do this, I will do them all at the same time.
Right now, the logic is that there is an underscore only after from, to, set, plot and return. Everything that follows is one word.
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:31
more underscores implies something like this: multitaper_cross_power_spectrum(). I don't necessarily have anything against that. Its longer, but easier to read.
also, with tab completion, I wonder if that would be annoying.
MMesch
@MMesch
Nov 07 2016 13:32
I don't think tab completion makes problems
another thing: why do you want to get rid of the get_
if so, we should do so consistently. I personally would keep it because I am used to get_ set_ methods
I have written my proposal for the function names in the PR
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:37
for me, powerspectrum() is a function. The method calculates it, and then returns it (i.e., it is not already stored in the class). On the other hand, something like set_ modifies the data stored in the class, and something like to_ takes the data out of the class and returns it in another (standard) format.
expand(), rotate() and powerspectrum() are all similar in that regard.
MMesch
@MMesch
Nov 07 2016 13:39
ok I see. to me powerspectrum() is more like accessing an attribute of the class
rotate() actually modifies it
expand() returns definitely not an attribute
get_ is used whenever an attribute is accessed. You are right that the power spectrum is not really an attribute, but I saw it as accessing a property of the coefficients, without modifying them
so everything that is just a property used to examine the coefficients or the grid uses get_
as in java; The terms get/set must be used where an attribute is accessed directly.
don't know how it is done exactly in python
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:43
I guess you could argue that what I renamed to degrees(), lats() and lons() is returning data stored in the attributes, and hence could be used with get_, but I just thought it was too verbose. On top of that, these are not explicitly stored, but calculated when called...
MMesch
@MMesch
Nov 07 2016 13:43
also I think I would want to have a verb in the function name to say what it is actually doing
that's just my personal opinion.
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:44
what about variable.conjugate?
MMesch
@MMesch
Nov 07 2016 13:45
you mean as a counter example?
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:45
Screen Shot 2016-11-07 at 14.45.28.png
MMesch
@MMesch
Nov 07 2016 13:46
that's a verb, right?
but you are right that it is like an attribute...
I don't know. Matter of taste maybe
I prefer having a verb to know exactly whether the function returns something or just processes some data internally
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:49
The problem for me is that all methods return something (even plot). The only exception is set_coeffs, and info().
MMesch
@MMesch
Nov 07 2016 13:53
rotate returns or does in place?
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:53
Here's another example: b.mean(), b.var(), where b is list.
MMesch
@MMesch
Nov 07 2016 13:53
hm ok
I am too java / c++ biased maybe :)
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:54
SHCoeffs.rotate() returns a new class instance. However, SHWindow.rotate() calculates the coefficients and saves them as a class attribute.
MMesch
@MMesch
Nov 07 2016 13:54
initially I thought that rotate would change in place, but then it doesn't make a lot of sense because a copy is made anyway
hm so I give up.
just a matter of taste :smile:
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:55
get_taper_number() : get_k()
MMesch
@MMesch
Nov 07 2016 13:56
I find number is not very explicit...
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:56
for this, it returns the number of tapers that have concenrtation factors greater than lamba (which is input). taper_number might be confused with a specific window.
I agree that number isn't great, nor was get_k.
MMesch
@MMesch
Nov 07 2016 13:57
get_optimal_taper_number ?
or optimal_taper_number
I think you convinced me with removing the get_, haha
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 13:58
number_tapers() is probably better than taper_number() imo.
MMesch
@MMesch
Nov 07 2016 13:59
with a hidden of ?
optimal_number_of_tapers
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 14:02
well, the original logic was that number() = number(0) = nwin
and that number(lambda) = number of windows with concentration factors greater than lambda....
MMesch
@MMesch
Nov 07 2016 14:03
oh ok I see. I think that might be to difficult to get.
At least for me :)
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 14:06
And you still prefer spectrum over power_spectrum ? I'm still not sure about that one.
20 min....
I didn't have autocompletion for vim :)
yes, I dislike power_spectrum if we return the energy_spectrum
MMesch
@MMesch
Nov 07 2016 14:40
any opinion from @ioshchepkov ?
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 15:44
We could also just remove the option energy and then keep power_spectrum. The difference between the two is only a factor of 4pi, and it could be argued that this optional argument might just cause confusion.
MMesch
@MMesch
Nov 07 2016 15:50
hm. I would absolutely prefer spectrum keeping energy. Otherwise we shut out all physicists and all seismologists that don't use 4pi convention usually.
just do unit='power_per_l' e.g.
or as you did it now
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 15:57
Ok. But the 4pi does not come from the normalization convention: it is from the definition that power is the function squared divided by the area of the sphere.
Here is another possibility. Instead having an optional argument energy=True or False, we could do the following:
clm.spectrum(convention='power' or 'energy')
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 16:18
In a similar manner, perhaps we could rename the triangle plot plot_power() to plot_coeffs(convention='power' or 'energy').
MMesch
@MMesch
Nov 07 2016 16:23
that's fine for me. Then we can use 'L2' as another convention
the default should be convention='power', unit='perl'
or something similar
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 16:34
so, should l2norm be a convention or unit?
probably convention. and l2 or l2norm?
rename SHWindow.number() to SHWindow.number_concentrated()
rename SHWindow.return_coeffs() to SHWindow.to_shcoeffs()
SHWindow.return_grid() to SHWindow.to_shgrid()
I think these are better too../
for plot_coeffs(), it doesn't make sense to have convention=l2norm, as we are only plotting a single coefficient. This is the only reason I can think of for having unit=l2norm
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 16:43
here is my compromise: SHTOOLS/SHTOOLS#69
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 16:57
minor modification: l2norm is now a convention. When specified, the units parameter is not used, given that we only plot/calculate sqaures of the coeffs. If we were to choose l2norm to be a unit, then convention could be neither power or energy.
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 17:09
last point: for symmetry reasons, I think that we should rename SHGrid.from_array() to from_grid(). The reason is that the SHWindow class needs to output both coeffs and grids as ndarrays, and we are using array and grid to distinguish the two there.
MMesch
@MMesch
Nov 07 2016 17:42
well unit for l2norm could be per_l or per_lm as well. I don't see why it shouldn't be valid ...
I find array better because it refers to numpy, I would rather change the Window class ?
not sure, I didn't look in detail.
I see what you mean though...
no idea how to resolve it cleanly ...
MMesch
@MMesch
Nov 07 2016 17:48
I don't understand plot_coeffs(), why don't we put it in the plot_spectrum() routine?
I think the plot_spectrum routine should just have an argument, as the unit argument, about the way how it is plotted: as a function of l or as a function of l and m
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 22:21
In my opinion, l2norm can't have units of per_l or per_lm, because the l2 norm is defined as the sum of the coefficients squared.
For SHWindow, we could conceivable remove the to_array() and to_grid() methods. This class already has to_SHCoeffs() and to_SHGrid(), so you could get the numpy array by SHWindow.to_SHCoeffs().to_array()
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 22:28
As for combining plot_spectrum() and plot_coeffs(), this could be done, but it might be complicated with all the optional parameters (there is power vs. l, and color vs. l vs. m.).
ok: updated proposal: SHTOOLS/SHTOOLS#69
Mark Wieczorek
@MarkWieczorek
Nov 07 2016 22:36
I suggest using SHGrid.to/from_array(), removing SHWindow.to_array/grid() and then keeping just SHWindow.to_SHCoeffs/SHGrid(). Last thing to resolve is plot_coeffs/spectrum(). Let me think about how to combine these two...