search

Goobley / Lightweaver

For the investigation of NLTE glowy stuff. A python framework for constructing solar NLTE radiative transfer simulations in one- and two-dimensional geometries, with an optimised C++ backend.
https://goobley.github.io/Lightweaver/
MIT License
17 stars 7 forks source link

Use astropy units #29

Open wtbarnes opened 3 years ago

wtbarnes commented 3 years ago

I know that this has come up in conversation at some point, but I think it would be really useful to use astropy units throughout this package, particularly at the user interface layer.

I fully realize that introduction of units is not a trivial addition and can also involve some performance penalties as well. However, the units on some inputs/outputs are sufficiently ambiguous to warrant this. For example, I was modifying the z component of the velocity on the FAL atmosphere and I was not sure what the components on the velocity were (I believe they are SI, m/s).

wtbarnes commented 3 years ago

Ah I've just found unit_view on Atmosphere. That is very useful!

Goobley commented 3 years ago

Indeed, the Atmosphere constructor can also take units too through the .make_*d statuc methods. Internally, everything is converted to SI, with the exception of wavelength in nm. This is discussed a little in the associated paper, which is supplementary to the docs (and probably the most code in an accepted ApJ article :D). This and the .unit_view were a compromise due to performance and a few issues I had with units behaving a little unexpectedly.

Something like .unit_view should probably be added to the spectral output too, and I'll leave this open to address this at some point, as well as improve the documentation.