-
Notifications
You must be signed in to change notification settings - Fork 626
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Normalize eigenmode source to unit power #728
Normalize eigenmode source to unit power #728
Conversation
…in the CW case); added discussion to FAQ section of documentation
I will add a test to verify functionality. |
python/source.py
Outdated
@@ -107,6 +109,10 @@ def __init__(self, | |||
self.eig_resolution = eig_resolution | |||
self.eig_tolerance = eig_tolerance | |||
|
|||
@eig_tolerance.setter | |||
def eig_tolerance(self, val): | |||
self._eig_tolerance = check_positive('EigenModeSource.eig_tolerance', val) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method seems to be duplicate of line 158.
…e_eigenmode_source_to_unit_power
…e_eigenmode_source_to_unit_power
Updated hard-coded comparison values for some python tests to accommodate revised normalization of eigenmode sources. Augmented I am not sure what is up with what appears to be a slight systematic offset between the curves. Maybe I misunderstand MEEP's conventions for the definition of frequency ranges. For the time being, the agreement at the center frequency is good and I have written the unit test to look at that. |
python/tests/mode_coeffs.py
Outdated
################################################## | ||
if nf>1: | ||
power_observed=mp.get_fluxes(mode_flux) | ||
freqs=[mode_flux.freq_min + n*mode_flux.dfreq for n in range(len(power_observed))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of computing freqs
manually, you can use the built-in routine get_flux_freqs
:
freqs=mp.get_flux_freqs(mode_flux)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I switched to this, and it now seems clear that the observed DFT data need to be plotted with a leftward shift of 0.001 (i.e. one-half the spacing between X tics) to fall correctly on top of the predicted data. In particular, the maximum entry in the 51-element flux()
array computed for the DFT cell is not the entry in cell #26, i.e. the exact middle of the array, but rather for cell #27, corresponding to a frequency slightly beyond the center frequency. I don't know what that is about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because get_flux_freqs
is based on Numpy's linspace
and is using endpoint=False
:
def get_flux_freqs(f):
return np.linspace(f.freq_min, f.freq_min + f.dfreq * f.Nfreq, num=f.Nfreq, endpoint=False).tolist()
Thus for an odd number of frequency bins, the center frequency will not be fcen
and will not be the middle element of the freqs
array (e.g., the 26th element of the 51-element array). Rather, the center frequency will be the element adjacent to the middle element (e.g., the 27th element of the 51-element array). For reference, this is demonstrated below.
>>> import numpy as np
>>> fcen = 0.20; df = 0.5*fcen; nfreq = 51;
>>> fmax = fcen+0.5*df; fmin = fcen-0.5*df;
>>> dfreq = (fmax-fmin)/(nfreq-1);
>>> freqs = np.linspace(fmin, fmin+dfreq*nfreq, num=nfreq, endpoint=False);
>>> print("26th element: {}".format(freqs[25]))
26th element: 0.2
>>> print("27th element: {}".format(freqs[26]))
27th element: 0.202
The size of the frequency spacing (i.e. dfreq
from above) is defined in fields::add_dft
.
From the Travis CI logs:
This is the output of Travis' |
I think this is a bug in the |
python/tests/mode_coeffs.py
Outdated
power_observed=mp.get_fluxes(mode_flux) | ||
freqs=mp.get_flux_freqs(mode_flux) | ||
power_expected=[source.eig_power(f) for f in freqs] | ||
import ipdb; ipdb.set_trace() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Travis is failing because of this line which should be removed (since it is presumably from debugging?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad.
* add --without-scheme option to configure to bypass building the scheme interface * revised normalization of eigenmode sources to yield unit power flux (in the CW case); added discussion to FAQ section of documentation * updates * update FAQ entry regarding normalization of eigenmode sources * updates * updates * added eig_power() method to EigenmodeSource class in python * updates * updates * updates * updates * updates * updates * augmented mode_coeffs.py unit test to check normalization of eigenmode sources * updates * updates * move docs from FAQ to manual, change fourier_transform to take f and not omega * delete blank line * tweak
Another stab at #470.