MPoL-dev / MPoL

A flexible Python platform for Regularized Maximum Likelihood imaging
https://mpol-dev.github.io/MPoL/
MIT License
33 stars 11 forks source link

Base unit for spatial frequency #223

Closed jeffjennings closed 4 months ago

jeffjennings commented 9 months ago

Is your feature request related to a problem or opportunity? Please describe. The default spatial frequency unit in MPoL is [klambda]. Nothing necessarily wrong with that, but it can be an unintuitive gotcha for users, and depending on which part of the code they're running, can be confusing to identify. It also necessitates putting factors of 1e3 and 1e-3 in many places, and complicates the handful of routines in the codebase that pull in functions from frank (which expect [lambda]). It would be a bit simpler if the base unit throughout the codebase were updated to [lambda], with only plotting routines modifying units.

Describe the solution you'd like Change the base unit to [lambda] throughout the codebase and in the docs.

iancze commented 9 months ago

After conversations with @jeffjennings and @j6626 yesterday, I agree we should do this. I thought klamba made a reasonable base unit for ALMA (most visibilities are 10's to 1,000's). But after a while working with this, I don't think klambda is any more intuitive than lambda in this context and has the actual downside of all those 1e-3 and 1e-3 factors you mentioned.

jeffjennings commented 9 months ago

Just noting this will also need to be done in visread (I think only here) since, as in #227, visread functions are being called internally in MPoL.