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

Unknown ufunc pmsafe from astrometry.py #1733

Closed
tcromartie opened this issue Apr 17, 2024 · 14 comments
Closed

Unknown ufunc pmsafe from astrometry.py #1733

tcromartie opened this issue Apr 17, 2024 · 14 comments

Comments

@tcromartie
Copy link
Contributor

I have provided the code that's failing (attached w/ nominal par, tim). When building an enterprise.pulsar Pulsar with 'pint' selected as the timing package (psr = Pulsar(par, tim, timing_package='pint')) and when PMRA/PMDEC are included in the model (not using 'tempo2' or when the astrometry parameters aren't included), it fails with the following:
TypeError: unknown ufunc pmsafe. If you believe this ufunc should be supported, please raise an issue on https://github.com/astropy/astropy
After the AstrometryEquatorial.ssb_to_psb_xyz_ICRS call to pmsafe in astrometry.py.
I can import erfa with no problem. erfa.pmsafe doesn't fail otherwise, though.
example.tar.gz

@tcromartie
Copy link
Contributor Author

I forgot to strip a lot of unnecessary imports etc. from the example. My apologies!

@dlakaplan
Copy link
Contributor

Can you be explicit about what versions you are using (astropy, erfa, pint, numpy, ...)?

@dlakaplan
Copy link
Contributor

Also, this sounds more like an enterprise issue/question. The pint-only commands work fine, right?

@tcromartie
Copy link
Contributor Author

I'm confused though because yes, I'm not having problems with pint commands. But this only happens when enterprise is set to use pint instead of tempo2. Enterprise is calling on pint's astrometry.py for some specific astrometric transformation that then in turn fails. I guess that's why it's not obvious to me that it's enterprise. (I'm not calling erfa directly at all, I only imported/printed it in the example to confirm it wasn't broken being invoked directly.)

pint 0.9.8+175.gc50630d7
enterprise 3.3.4.dev13+gc49646c
np 1.23.5
astropy 6.0.0

I can paste the traceback if needed.

@dlakaplan
Copy link
Contributor

Can you either find what the enterprise command is, or ask @Hazboun6 ?

@tcromartie
Copy link
Contributor Author

tcromartie commented Apr 17, 2024

You mean my actual enterprise call? Just what's in the example code, psr = Pulsar(par, tim, timing_package='pint'). Enterprise builds a pulsar through pulsar.py and when pint is specified, it uses the class PintPulsar.

Within that, this causes the problem:
self._pos_t = model.components[which_astrometry].ssb_to_psb_xyz_ICRS(model.get_barycentric_toas(toas)).value'

And the failure is in pint's astrometry.py: AstrometryEquatorial.ssb_to_psb_xyz_ICRS(self, epoch), when starpmout = pmsafe(...) is defined (:446)

@dlakaplan
Copy link
Contributor

So this is an enterprise error. The usage for ssb_to_psb_xyz_ICRS says that the argument should be:

epoch : float or astropy.time.Time, optional

but enterprise is trying to pass a plain Quantity. Something like:

model.components[which_astrometry].ssb_to_psb_xyz_ICRS(Time(model.get_barycentric_toas(toas),format='mjd')).value

will work instead, I think.

@dlakaplan
Copy link
Contributor

Or (and maybe this was the intent):

model.components[which_astrometry].ssb_to_psb_xyz_ICRS(model.get_barycentric_toas(toas).value)

@dlakaplan
Copy link
Contributor

Note that it's possible to support quantity input, it just hasn't come up.

@tcromartie
Copy link
Contributor Author

Thanks very much @dlakaplan, I appreciate your help!

@tcromartie
Copy link
Contributor Author

(Confirmed that the change model.components[which_astrometry].ssb_to_psb_xyz_ICRS(model.get_barycentric_toas(toas).value) is successful. Thank you!)

@Hazboun6
Copy link
Member

I agree that you should submit this as an Enterprise PR @tcromartie ! I am surprised this hasn't popped up before though, since this is in the __init__ method of the PintPulsar class.

@vhaasteren
Copy link
Member

vhaasteren commented Apr 18, 2024

@dlakaplan, @tcromartie, @Hazboun6

I am not sure whether I agree that this only an Enterprise issue. It's an inconsistency in the PINT API as well

For Ecliptic coordinates, if you do m.components['AstrometryEcliptic'].ssb_to_psb_xyz_ICRS, then units in the form of quantities are supported.

For Equatorial coordinates, the units are not supported. Of course it's easy to change in Enterprise, but IMO this should be made consistent in PINT as well. I don't like Enterprise passing the .value to PINT, as PINT is supposed to be units-aware, so passing them is preferred. @dlakaplan's suggestion of using this would be best?

model.components[which_astrometry].ssb_to_psb_xyz_ICRS(Time(model.get_barycentric_toas(toas),format='mjd')).value

The reason this hasn't come up is because the only PTAs using PINT have been using Ecliptic coordinates.

@dlakaplan
Copy link
Contributor

I think that using the quantity versions isn't actually supported, but works accidentally (the docstrings aren't all fully consistent, but don't claim to support units). However, I agree that they should.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants