Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slicing TransientSpec #204

Open
jlaehne opened this issue Jan 1, 2024 · 22 comments · May be fixed by #205
Open

Slicing TransientSpec #204

jlaehne opened this issue Jan 1, 2024 · 22 comments · May be fixed by #205
Labels
type: feature request Asking for additional functionality

Comments

@jlaehne
Copy link
Contributor

jlaehne commented Jan 1, 2024

When slicing, summing, integrating signals of type TransientSpec, the resulting signal will be a Signal1D. It would be desirable to obtain signals of type Transient or Luminescence depending on what axis the function operates on.

@jlaehne jlaehne added the type: feature request Asking for additional functionality label Jan 1, 2024
@ericpre
Copy link
Contributor

ericpre commented Jan 2, 2024

This touch on the fact that the LumiTransientSpectrum doesn't fit well with the hyperspy signal paradigm: this class have signal axes with signal type of different nature and when there is a dimensionality reduction of the signal dimension, the signal will be mapped to a class of the same signal_type, which is not possible because there is no 1D signal counterpart.

To fix this, here are two options:

  • keep the existing class structure and find a mechanism to patch all function with dimensionality reduction in the LumiTransientSpectrum class.
  • Remove the LumiTransientSpectrum and set the energy/wavelength axis to a navigation axis.

The first option defies the hyperspy signal paradigm and I suspect that this will be very messy. The second is clean and simple.

@jlaehne
Copy link
Contributor Author

jlaehne commented Jan 2, 2024

Well, the issue is that TransientSpec basically inherits from two different Signal1D classes for the two axes and HyperSpy so far does not support two different signal types for a single object. A third (and my prefered) solution would be to expand the HyperSpy signal paradigm and allow for something like signal_type being a list.

The issue is that neither the energy/wavelength nor the time axis should be a navigation axis. We can have linescans or maps of both types of 1DSignals, as well as of this 2DSignal and I would find it even more confusing to then add a non spatial dimensionality to navigation.

The essence is that this type of signal is a new use case that so far was not covered by hyperspy. If a signal had multiple signal dimensions, these were always of the same type.

@ericpre
Copy link
Contributor

ericpre commented Jan 2, 2024

The issue is that neither the energy/wavelength nor the time axis should be a navigation axis. We can have linescans or maps of both types of 1DSignals, as well as of this 2DSignal and I would find it even more confusing to then add a non spatial dimensionality to navigation.

This is usual to have non spatial dimension as navigation dimension, these can be time, stage tilt, energy (for example energy filtered TEM), angular dependence of diffraction pattern, etc.

The essence is that this type of signal is a new use case that so far was not covered by hyperspy. If a signal had multiple signal dimensions, these were always of the same type.

The motivation behind the core concept of splitting dimensions between navigation and signal dimension is to be able to naturally operate on the signal dimension and map it to all (or a subset of the) navigation axes and vice versa for operation applied on navigation axes. When there is something special to do about a specific axis, this can be customised using transpose, etc. - see for example, the in situ diffraction data (pyxem/pyxem#900), where time correlation analysis has been implemented. @CSSFrancis may be able to comment more on this.

Maybe it would be useful to have an example of operations that needs to operate on several signal axes of different signal type to justify having together in the signal space?

@jlaehne
Copy link
Contributor Author

jlaehne commented Jan 2, 2024

There is a number of reasons, why I would like to have both as signal axes (and why it is not comparable to a stage tilt or angular dependence series):

  1. We will have the use case of maps of streak camera images, and we would get a mixture of axes types in the navigation dimensions.
  2. People are used to display/view streak camera data as images - if one of the axes is a signal axis and one a navigation axis, hyperspy would plot the data differently than expected.
  3. We have both types of axes as different signals recorded with dedicated detectors (CCDs for spectra and time-correlated single photon counting for transients).
  4. For both types of signals, the modelling functionality will be used.

As a consequence, the two axes are of equal relevance and I do not see which of the two should become the navigation and which the signal axis.

Probably the most common use case will be that people use slicing to reduce the image to either time or wavelength dimensions by integrating over certain intervals of the other axis and then model the 1D data. However, doing so both ways for the same image!

Changing the hyperspy paradigm and allowing different signal_types within one class might be the more universal approach, but without other use cases, it would indeed be a tedious task to implement that properly.

Writing wrappers in LumiTransientSpectrum for all functionalities with dimensionality reduction that handle the assignment of the correct signal_type might then be the most feasible way to go.

@ericpre
Copy link
Contributor

ericpre commented Jan 2, 2024

Point 1 to 3 seems fine to me (or will need small changes) using current hyperspy. Having navigation axes with different type (spatial, time, etc.) is usual.
For point 4, are you thinking of 2D model? Currently, there is only Gaussian2D, so I assume this would be two different sigma for the two different axes. If the only situation where you both axes to be in the signal axes, this can be done by transposing the axes and create the model in a workshop.

As a consequence, the two axes are of equal relevance and I do not see which of the two should become the navigation and which the signal axis.

One way to addressed this is by being explicit and documenting how to convert from one to another and what does it mean for visualisation and processing.

@jlaehne
Copy link
Contributor Author

jlaehne commented Jan 2, 2024

For point 4, are you thinking of 2D model? Currently, there is only Gaussian2D, so I assume this would be two different sigma for the two different axes. If the only situation where you both axes to be in the signal axes, this can be done by transposing the axes and create the model in a workshop.

A Gaussian2D would not be appropriate for the type of data - but so far I am not aware of anyone actually modeling that in both dimensions. Instead, it is common to model and plot individual slices (at different times or different wavelength ranges) to evaluate the evolution of the data.

@CSSFrancis
Copy link

@jlaehne This is an interesting point. My opinion on this is strongly correlated with performance of the map function. Where the signal axes should be the ones you are actively operating on and navigation axes should be everything else.

In this case it seems like you might have two signals with one being the transpose of the other if you want to operate on both separately. If you want to do as @ericpre suggests fitting a 2D function to a 2D image actually sounds like it would be an interesting thing to try.

What about supporting convolving 1D models to create 2D models?

In the long run we could make the map function operate on any axes but I would still use that as an exception rather than the rule.

@jlaehne
Copy link
Contributor Author

jlaehne commented Jan 27, 2024

In 0d2c439 (#205), I have now implemented the following:

  • LuminescenceTransientSpectrum is now getting signal_type = 'Luminescence', and is thus automatically chosen when signal_dimension = 2
  • When slicing to one dimension, it thus directly takes the 1D Luminescence class
  • To handle the time (transient) dimensions, if Luminescence is initialized in 1D with axes_manager[-1].units being a time unit ('s', 'ps', ...) switch to LumiTransient class

This way, all functions reducing the dimensionality seem to work correctly (just have to add the formal tests).

However, having a Luminescence class both in 1D and 2D entails a problem for the mapping function that currently lets some tests fail. When operating on a map of 1D Luminescence-signals, so that the result is a 2D image in the navigation dimensions, a LuminescenceTransientSpectrum object is returned, when one would want to have a Signal2D onject, because the signal axes are now actually spacial units and not wavelength/time. An example:

>>> import hyperspy.api as hs
>>> from lumispy.signals import LumiSpectrum
>>> s = LumiSpectrum([[[1, 2, 3, 4, 5]] * 3] * 4)
>>> com = s.centroid()
<LumiTransientSpectrum, title: Centroid map, dimensions: (|3, 4)>

@CSSFrancis do you have an idea how to fix that behaviour?

@CSSFrancis
Copy link

@jlaehne hmmm. So the map function will automatically keep the signal type of possible. So if you have the same alias it will just cast it.

I had plans for allowing the axes and signal type to be set with the map function but never got around to it.

Currently we just have lots of code that just resets the axes and signal type after the map function.

@jlaehne
Copy link
Contributor Author

jlaehne commented Jan 28, 2024

I could of course just set the signal type explicitly in centroid, but it also applies to hyperspy functions such as estimate_peak_width.

Alternatively, I guess I could go back to having the 2D signal type TransientSpec and instead introduce a 1D equivalent for this one that only serves for casting to either Luminescence or Transient when slicing a TransientSpec. For the moment, this actually seems like the cleaner solution to me.

@jlaehne
Copy link
Contributor Author

jlaehne commented Jan 28, 2024

Alternatively, I guess I could go back to having the 2D signal type TransientSpec and instead introduce a 1D equivalent for this one that only serves for casting to either Luminescence or Transient when slicing a TransientSpec. For the moment, this actually seems like the cleaner solution to me.

This works (9b54af6), and it seems a decent solution! The only downside is that I have to register this additional class TransientSpectrumCasting (which is meant only for the decision between different 1D classes) with HyperSpy and it will be listed with print_known_signal and in the hyperspy-extensions-list. Do we have an option or would it be feasible to have "hidden" signal classes @ericpre ?

@jlaehne
Copy link
Contributor Author

jlaehne commented Jan 29, 2024

The only downside is that I have to register this additional class TransientSpectrumCasting (which is meant only for the decision between different 1D classes) with HyperSpy and it will be listed with print_known_signal and in the hyperspy-extensions-list. Do we have an option or would it be feasible to have "hidden" signal classes @ericpre ?

Would be allowed with hyperspy/hyperspy#3302 by defining an optional, additional key hidden when registering an extension signal to simply exclude them from print_known_signal_types.

@jlaehne jlaehne linked a pull request Jan 29, 2024 that will close this issue
7 tasks
@ericpre
Copy link
Contributor

ericpre commented Feb 2, 2024

I really think that having a 2D signal with two different type of signal (here: time and energy) is what cause the issue/confusion here. As discussed above, there isn't any processing where this mixed signal is necessary. On top of that, some of the hyperspy.api.signals.Signal2D method will broken (calibrate, align2D/estimate_shift2D). For these reasons, I think trying to cast these two dimensions of different nature to the same signal space is not the right approach and that there isn't a use case.
When the transcient/spectral data are acquired in a 2D fashion on a camera, at loading time it should be parsed to a 1D signal and then transpose should be used in the workshop.

What is the issue with using a 1D signal (with either time or energy/wavelength as signal dimension) and transposing when necessary at the workflow level?

@jlaehne
Copy link
Contributor Author

jlaehne commented Feb 3, 2024

Well, the first thing one usually does with the data is to plot it for visualization and the expected way to look at the streak camera data is an image with a time and a wavelength axis. For linescans and maps I would then expect a navigator that allows me to go through the different images. That is why I would want the 2D-signal as primary object.

Besides the plotting, I do not see any of the two axes as "preferred" signal axis to which the object should be initialized. Instead, in the analysis one usually does certain operations on the one and certain operations on the other axis.

Thus, when wanting to operate on one of the signal axes, one could transpose to a single signal axis, e.g. by s.transpose(signal_axes=['Time']) or s.transpose(signal_axes=['Wavelength']). (for which one could discuss having shorthand wrappers like s.time() and s.lumi()).

With the current implementation, transposing to either of the signal axes, as well as using any dimensionality reducing command such as sum, max, integrate1D, isig, ... casts the class to the correct 1D class, which I find an elegant solution for handling this type of data.

@ericpre
Copy link
Contributor

ericpre commented Feb 3, 2024

Well, the first thing one usually does with the data is to plot it for visualization and the expected way to look at the streak camera data is an image with a time and a wavelength axis. For linescans and maps I would then expect a navigator that allows me to go through the different images. That is why I would want the 2D-signal as primary object.

It should be possible to do that by improving the plotting, particularly with hyperspy/hyperspy#3199.

Besides the plotting, I do not see any of the two axes as "preferred" signal axis to which the object should be initialized. Instead, in the analysis one usually does certain operations on the one and certain operations on the other axis.

Yes, but at some point, I would imagine that you need to make that decision to process the data, as there is no processing that you can do when the data is in that 2D signal structure?

Thus, when wanting to operate on one of the signal axes, one could transpose to a single signal axis, e.g. by s.transpose(signal_axes=['Time']) or s.transpose(signal_axes=['Wavelength']). (for which one could discuss having shorthand wrappers like s.time() and s.lumi()).

With the current implementation, transposing to either of the signal axes, as well as using any dimensionality reducing command such as sum, max, integrate1D, isig, ... casts the class to the correct 1D class, which I find an elegant solution for handling this type of data.

Yes, but it breaks some of the basic usage of Signal2D, e.g. calibrate, align2D/estimate_shift2D? Fitting will not work. What's about machine leaning? I would expect that the basic denoising usage will not work neither.

I think that there is a solution which doesn't break Signal2D, and what is needed here is to improve the visualisation. I am happy to have a look at the feasibility if this helps.

@jlaehne
Copy link
Contributor Author

jlaehne commented Feb 3, 2024

@gkusch, @Attolight-NTappy you have quite some experience working with streak camera data, so it would be interesting to get your oppinion here.

@Attolight-NTappy
Copy link
Contributor

Hi everyone, happy to give my opinion.

I am of @ericpre 's opinion that LumiTransientSpectrum could be removed and time/spectrum axis moved to navigation dimension. What @jlaehne wants to do in initial post is already easy. Example using a streak dataset openly available from https://zenodo.org/records/6384277 (sorry for referencing my own work, I use what I know is readily available), with hyperspy 2.0 and lumispy 0.2.2:

import hyperspy.api as hs
s = hs.load("Streakmap_P4_-1-0_-1-0.hspy") 

"""Casting into TransientSpec"""
s.set_signal_type('TransientSpec')
s
# <LumiTransientSpectrum, title: , dimensions: (|672, 508)>

"""Summed signal casting into desired signal type"""
s2 = s.sum(0)
s2.set_signal_type('Transient')
s2
# <LumiTransient, title: , dimensions: (|508)>

Why should anything be changed to save a benign call to set_signal_type ?

Additionally, there is currently no functionality implemented in LumiTransient over Signal1D. Neither does LumiTransientSpectrum implement added functionality over LumiSpectrum. The reasons for preferring either ones over the others are unclear to me.

I also think that most time-resolved spectra are primarily treated as a succession of spectra during analysis and do not require functionality from Signal2D. Quite the contrary, when dealing with streak maps the first thing I usually do is applying s = s.as_signal1D(0). Loading streak maps in Signal1D (or subclass) should simply be the preferred option.

@CSSFrancis
Copy link

It should be possible to do that by improving the plotting, particularly with hyperspy/hyperspy#3199.

It would be good to make sure things there are working with this case? I think you are right that is the best solution.

@Attolight-NTappy
Copy link
Contributor

@CSSFrancis continuing on the previous example

import hyperspy.api as hs
s = hs.load("Streakmap_P4_-1-0_-1-0.hspy") 
s.set_signal_type('TransientSpec')
s.as_signal1D(0).plot()

already yields a map as a navigator, see below:
image

don't understand what is the desired change in behaviour.

Cheers

Nicolas

@CSSFrancis
Copy link

@Attolight-NTappy that is a single streak dataset for one position correct? If you wanted to visualize a 2-d map of streaks you would want a map of intensity in real space and then the set of streaks at each position. Correct?

I've never done any of these experiments so I'm not really sure what I'm looking at 😅

@LMSC-NTappy
Copy link
Contributor

@CSSFrancis

that is a single streak dataset for one position correct?

Absolutely.

If you wanted to visualize a 2-d map of streaks you would want a map of intensity in real space and then the set of streaks at each position. Correct?

Yes. Depending on how the data dimensions are arranged between navigation and signal spaces, you would need to use as_signal2D and set the correct axes in signal space or not. Not sure how a signal of the <(a,b,c|d)> type natively plots, but the point is that getting the desired behavior is not difficult.

Not sure how common 2d-scans of streak maps are, but that's an entirely different question ;]

@jlaehne
Copy link
Contributor Author

jlaehne commented Feb 29, 2024

Thanks everyone for the thoughts, trying to get back into the discussion:

Even if measurement time surely limits their frequent occurence, 1d and 2d scans of streak camera images should be covered - after all efficiently working with these types of multidimensional data is exactly what HyperSpy is for.

Of course, current functionality of classes like LumiTransient is still missing, and manual signal casting is just a one-liner - but the idea for the moment is to set the framework to then develop more elaborate and specific functionalities like models for the transients. Similar to the energy conversion that is available for luminescence spectra. And to do that in a way that it will still work in higher dimensions.

For TransientSpec, many Signal2D routines might indeed not work, but certain functionality could be interesting, e.g. finding a peak/maximum over the overall image and not only over individual axes. However, for most analyses the signal will indeed be cast into its one dimensional representations as in the example from @Attolight-NTappy above.

The main point with streak camera images is that the time and wavelength axes are somewhat of equal importance. It is not simply a stack of spectra at different times, but at the same time a series of transients at different wavelengths. In an analysis, one will often not only look at either of the two, but from the same dataset extract both spectra integrated over a certain range of times, as well as transients over a certain spectral range. One could for example track the position in wavelength of a peak over time, while also looking at the lifetimes extracted from the transients as a function of wavelength.

With a TransientSpec signal that inherits both from the transient and spectrum classes, certain functionalities of either class can be performed on the overall map, such as converting the wavelength to energy prior to further slicing or analysis.

Therefore, I still see the two axes as equally important, and choosing one as the primary signal axis and the other as a navigation axis seems somewhat arbitrary to me. And was, besides for visualization, a reason for me to go for the implementation of the TransientSpec class. In that framework, #205 is an implementation for the proposed automatic casting.

Looking at the analysis in the zenodo repo shared by @Attolight-NTappy above, it does indeed do exactly the same as proposed here, first work with the streak image as Signal2D and later cast it into a Signal1D (just using the generic hyperspy functions).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request Asking for additional functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants