Projection/slicing returns unyt_array instead of cosmo_array

[bwvdnbro]

The projection and slicing functions return an unyt_array instead of a cosmo_array. Because of this, the conversion from co-moving to physical coordinates needs to be done manually. This is also a bit misleading, since this gives the impression that the resulting maps are in physical coordinates, while they are not if the coordinates in the snapshot are co-comoving.

Is there any particular reason for this? Can we just replace unyt_array with cosmo_array to fix this or does that cause other issues?

[JBorrow]

You can do that, but you'll have to be careful to copy over the metadata. I was also not so sure what to do in the case of comoving/physical conflicts. The length scales are usually all co-moving, but the properties may be physical. You'd also likely want to make a check that hsml and the length scales are all in the same frame.

[bwvdnbro]

I discussed this with @MatthieuSchaller and we think the best option would be to just throw an error if someone tries to use physical and co-moving quantities together.

But I guess what you are saying is that a quantity like "mass" can be physical and still be meaningfully used together with co-moving length scales? So simply checking that all quantities involved have the same value for the comoving flag (and the same cosmo_factor) is not sufficient?

[MatthieuSchaller]

How about everything should be:

• "physical or transform as a^0" and then return the data in physical
• "comoving or transform as a^0" and then return the data in comoving

Error if not in either of these.

[JBorrow]

Ok, in that case this could be accommodated. It might be a nice test of the system too, if you make everything in comoving and convert it to physical densities too... I'd like to see good test coverage here if this feature is added.

[kyleaoman]

If this is implemented in terms of numpy functions, then #219 (which I should probably re-title...) will probably resolve this organically - it tries to adopt a sensible strategy to combine cosmo_factors when possible, warn when there is ambiguity, and raise when there is conflict. Basically extending what we're already doing for numpy ufuncs to all of numpy. It would be useful to have a test case (not necessarily a pytest, just a snippet to run to get started) so that I can see whether the changes I'm making help here.

If there are key scipy functions used those could be supported. Currently have no ambition to support all of scipy because (1) unyt has limited support for it and (2) each function and even many parameter combinations needs to be considered and implemented individually (only 50 more to go to cover everything in numpy that unyt does 🤪).