AstarVienna / ScopeSim

A telescope observation simulator for Python.
GNU General Public License v3.0
16 stars 10 forks source link

question/suggestion: Why deal with fits at all in scopesim? #131

Open krachyon opened 2 years ago

krachyon commented 2 years ago

Following the discussion today, if ScopeSim is intended as a "give me the pixel-array this telescope would see", to my mind it does not make sense to (have to) deal with fits/HDUs at all. I.e. something like micado.readout() should return a np.ndarray, not a hdulist. In case

I'd suggest decoupling the functionality in such a way: Have an API that does only the image generation. If the incorporation of fits/header keyword IO is needed, have that component use the image API and add the metadata.

hugobuddel commented 2 years ago

An HDU-list is not that different from a list of numpy arrays right?

The 'instrument' provides much more than just pixels. There are many sensors that are read out and that are used by scientists and in the data reduction software. So those need to be simulated by ScopeSIM as well (at least in theory). Doing that as a metadata in fits seems like a reasonable approach because that mimics what is provided by the real instrument (or instrument control software).

This is a also discussion about where ScopeSIM should stop. E.g., should it also simulate the Detector Control Software? Yes, because that includes various hardware effects too. Should it also include the Instrument Control Software? Yes, because the primary users of ScopeSIM are scientists and the data processing group, and they expect that.

But ScopeSIM can do both indeed, so ultimately yeah, I agree with you. Practically, I don't mind this implementation as much. (Reading what I write makes me wonder how I learned to stopped worrying and love [the] FITS...)

Perhaps the make-this-a-fits shoudl be a separate effect that is included by default. So nothing special has to be done, readout() will either include that effect or not.

krachyon commented 2 years ago

An HDU-list is not that different from a list of numpy arrays right?

While the interface is indeed similar, it does mix the concern of IO according to a specific format into a functionality that could just as well use idiomatic python data-structures (list/array/dict or possibly something like a dataclass for the header information to make it explicit/discoverable). I'd turn this around and say there's no reason to use a HDU-list if you can use something with fewer strings attached and convert it as soon as someone actually wants to write it to disk.

This is a also discussion about where ScopeSIM should stop. E.g., should it also simulate the Detector Control Software? Yes, because that includes various hardware effects too. Should it also include the Instrument Control Software? Yes, because the primary users of ScopeSIM are scientists and the data processing group, and they expect that.

Eh, do I really care about the details of how to address the servos to point the instrument? Probably not, at least I should not unless I specifically ask for it. But I guess it does depend on what abstraction layers you see as the domain of ScopeSim, which different users will obviously have different perspectives on. From the meeting on Friday it did indeed sound like metadata and details of the electronics where not part of the original plan at least.

So again, I'd strive for a clean separation of concerns here to avoid a system where you have everything very tightly coupled and need to know/care about details that you don't really want to interact with. E.g. I really want a high precision astrometry mode for what I did so far, but if you want to look at how the detector linearity changes with temperature you couldn't care less (and vice versa). In a system where both are separate modules you could still support an interface that ties it all together, but you give users the choice to pick the level of abstraction/realism they want.

hugobuddel commented 2 years ago

OK yes, you are right. It would be nicer to have some default numpy/astropy data structures and only map those to FITS when desired. FITS is so annoying, e.g. #136.

Perhaps there should be an extra object class (using ScopeSIM terminology, next to atmosphere, telescope, instrument, detector) that adds metadata and converts to the desired format. Where for now we would only need the FITS format, because omitting the effect altogether would return Python structures. Later perhaps we could also create png's, HDF5, whatever.

I don't think this is urgent enough to do soon, as there are suitable workarounds.

astronomyk commented 2 years ago

Hi guys,

I'll just jump in with a quick remark tonight and add more tomorrow - I don't want to be using astropy.fits objects simply because they are slow and full of annoying checks and recursive calls. At some point 2 years ago I wrote the PoorMansHeader class (in fov_utils) to speed up the spectroscopy mode. It has been (low) on my to-do list to write a PoorMansFITS class that has a similar api to the astropy.fits objects, but that is simply a dict+np.ndarray.

So yes, one day when I have nothing to do, I'll get ScopeSim off its addiction to the FITS format and the astropy fits module. But unfortunately today is not that day.

Hugo Buddelmeijer @.***> schrieb am Mo., 11. Apr. 2022, 21:20:

OK yes, you are right. It would be nicer to have some default numpy/astropy data structures and only map those to FITS when desired. FITS is so annoying, e.g. #136 https://github.com/AstarVienna/ScopeSim/issues/136.

Perhaps there should be an extra object class (using ScopeSIM terminology, next to atmosphere, telescope, instrument, detector) that adds metadata and converts to the desired format. Where for now we would only need the FITS format, because omitting the effect altogether would return Python structures. Later perhaps we could also create png's, HDF5, whatever.

I don't think this is urgent enough to do soon, as there are suitable workarounds.

— Reply to this email directly, view it on GitHub https://github.com/AstarVienna/ScopeSim/issues/131#issuecomment-1095463705, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACH66QU6MWP7EFMYOFFFM3LVER3N3ANCNFSM5S5BV6BA . You are receiving this because you are subscribed to this thread.Message ID: @.***>