ziotom78 / Healpix.jl

Healpix library written in Julia
GNU General Public License v2.0
51 stars 18 forks source link

type instability in documentation #79

Closed xzackli closed 2 years ago

xzackli commented 2 years ago

Would you be ok with removing the section in the documentation that describes a union type array for replacing UNSEEN?

https://github.com/ziotom78/Healpix.jl/blob/71f202030c62e96a4df5abd6ed3bae71478fba4b/docs/src/mapfunc.md?plain=1#L81-L85

People should not do this, because any code that operates on such a map becomes type unstable -- the compiler is unlikely to specialize code (i.e. for floating point operations) because the type of an element must be inferred at runtime. Julia fundamentally cannot run much faster than an interpreted language like Python for such arrays.

ziotom78 commented 2 years ago

Mmm… I see your point, but I must say that it's still an handy feature when working in a Pluto notebook, and I regularly show it to those students of mine that are using Healpix.jl. There are cases where it is better to use elegant code rather than fast, for instance when the bottleneck is not in looping over pixels.

Perhaps it would be better to rephrase the paragraph and add a big warning in the docstring.

ziotom78 commented 2 years ago

Hi @xzackli , I added a warning with some benchmarks in the documentation, hope this is ok.