Skip to content
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

Luminosity unit conversion obs_bandwidth #14

Closed
telegraphic opened this issue Apr 1, 2019 · 7 comments
Closed

Luminosity unit conversion obs_bandwidth #14

telegraphic opened this issue Apr 1, 2019 · 7 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@telegraphic
Copy link
Contributor

Hey, was playing with luminosity conversion and I'm getting a unit error:

frb = fruitbat.Frb(name='FRB170922', gl=45.07, gb=-38.70, fluence=177, dm=1111,
                   obs_bandwidth=31.25)
frb.calc_dm_galaxy()
frb.calc_redshift()
print(frb.calc_energy())

>>> 1.7065900447006508e+41 erg

But with units on obs_bandwidth:

from astropy import units as u

frb = fruitbat.Frb(name='FRB170922', gl=45.07, gb=-38.70, fluence=177, dm=1111,
                   obs_bandwidth=31.25*u.MHz)
frb.calc_dm_galaxy()
frb.calc_redshift()
print(frb.calc_energy())

>>> [traceback here]
UnitConversionError: 'Jy MHz2 Mpc2 ms' (power) and 'erg' (energy) are not convertible

Also - smallish feature request: compute average luminosity in ergs/s

@abatten
Copy link
Owner

abatten commented Apr 1, 2019

@telegraphic The error when using units on obs_bandwidth is because it add units to bandwidth by default similar to DM and fluence. So when specifying units when instantiating the class Frb will lead to a value of 31.25 MHz**2 when doing internal calculations. Hence the units are non-convertible. I think adding a check to the setters in the class Frb to see if the correct units are already assigned would resolve this issue. I'll open this as an issue for enhancement.

As for the average luminosity, I have a beta version of a calc_luminosity() function already developed but not released yet, however I found various differing definitions in the literature so I wasn't sure exactly which one to implement. Do know the preferred reference for calculating luminosity, if so I can add it relatively quickly.

@abatten abatten added bug Something isn't working enhancement New feature or request labels Apr 1, 2019
@telegraphic
Copy link
Contributor Author

I've just been dividing energy by width ;)

Wael just reminded me that in the Zhang (2018) paper they picked up that central observing frequency nu should be used instead of obs_bandwidth to estimate isotropic energy (I'm checking some of his calculations against fruitbat!):

The (1+z) factor was misused in the expression of E in some previous papers. The central frequency νc, rather than the bandwidth B, should be used in these calculations.

It's this equation:
Screen Shot 2019-04-01 at 6 16 26 pm

@abatten
Copy link
Owner

abatten commented Apr 1, 2019

So are you suggesting that both calc_energy() and calc_luminosity() should use a new parameter obs_freq_central instead of obs_bandwidth? This should be a pretty quick change.

@telegraphic
Copy link
Contributor Author

Yes, that's my understanding, they suggest that obs_freq_central is more appropriate. (I wouldn't be suprised if the argument continued though, as some pulses may be inherently band-limited, and others may be much wider band than the bandwidth of the receiver).

@abatten
Copy link
Owner

abatten commented Apr 2, 2019

What I am going to do is use obs_freq_central by default and have a keyword use_bandwidth in the function call for calc_luminosity() and calc_energy() that will allow for the option of using obs_bandwidth.

abatten added a commit that referenced this issue Apr 2, 2019
* Added a `calc_luminosity()` function that takes a parameter
`use_bandwidth` to choose between bandwidth and central observing
frequency.
* Modified `calc_energy()` to now use `obs_freq_central` by default
instead of `obs_bandwidth`
* Added checks on units passed to Frb. If the units are incompatible it
will return an error, otherwise it with use the units specified rather
than the default.
* Added tests for new luminosity and energy functions.
* Added tests for correct and incorrect units in Frb.
@abatten
Copy link
Owner

abatten commented Apr 3, 2019

The pull request #16 updating to version 0.2.1 has the luminosity function and passing units updates.

You can use the calc_luminosity() function with either obs_freq_central or obs_bandwidth. The same goes for calc_energy() however it now uses obs_freq_central by default.

I also made it possible to pass values with astropy units when calling Frb() and it will use those units instead of the predefined units I gave them unless the units aren't convertible.

If this works as you expect let me know and I can close this issue.

@abatten abatten self-assigned this Apr 3, 2019
@abatten
Copy link
Owner

abatten commented Apr 26, 2019

I'm closing this issue based on talking to you in person, the issue seems resolved.

@abatten abatten closed this as completed Apr 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants