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

Detector efficiency (replaces #482) #2535

Open
wants to merge 14 commits into
base: RELEASE_next_minor
Choose a base branch
from
Open

Detector efficiency (replaces #482) #2535

wants to merge 14 commits into from

Conversation

tjof2
Copy link
Contributor

@tjof2 tjof2 commented Sep 4, 2020

Description of the change

Replaces #482 and brings it up-to-date with Rnm. Ideally someone else should take this over, not my area of expertise. Looks like @sem-geologist and @ZanettaPM commented on #482 and may be able to contribute - and #1835 is seemingly related.

Compute the detector efficiency from the layers. The efficiency is calculated by estimating the absorption of the different the layers in front of the detector.

        >>> s = signals.EDSTEMSpectrum(np.ones(1024))
        >>> s.axes_manager.signal_axes[0].scale = 0.01
        >>> s.axes_manager.signal_axes[0].units = "keV"
        >>> s.detector_efficiency_from_layers()
        <EDSTEMSpectrum, title: Detection efficiency, dimensions: (|1024)>

Progress of the PR

  • Change implemented (can be split into several points),
  • update docstring (if appropriate),
  • update user guide (if appropriate),
  • add tests,
  • ready for review.

@codecov
Copy link

codecov bot commented Sep 4, 2020

Codecov Report

Merging #2535 into RELEASE_next_minor will increase coverage by 0.00%.
The diff coverage is 77.41%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##           RELEASE_next_minor    #2535   +/-   ##
===================================================
  Coverage               75.54%   75.54%           
===================================================
  Files                     202      202           
  Lines                   29534    29565   +31     
  Branches                 6436     6441    +5     
===================================================
+ Hits                    22312    22336   +24     
- Misses                   5402     5406    +4     
- Partials                 1820     1823    +3     
Impacted Files Coverage Δ
hyperspy/_signals/eds.py 77.92% <63.15%> (-0.77%) ⬇️
hyperspy/misc/eds/utils.py 85.89% <100.00%> (+0.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 60610f4...26d5123. Read the comment docs.

@pquinn-dls
Copy link
Contributor

If interested in supporting XRF signal it's not general enough.
Ge detectors are still popular but this method only uses Si as the detector material.
In terms of units cm is standard in x-ray applications.
Doesn't seem to deal with compounds - e.g. an air path.
I have code to do this so will suggest something. Many x-ray applications use xraylib to handle elemental properties and x-ray properties related to EDX/XRD. Would there be support for moving across to this ?

@ericpre
Copy link
Member

ericpre commented Oct 24, 2020

I am not knowledgeable on the details of this topic and I will not be able to comment sensibly on this.
In term of distribution, xraylib should be added as an optional dependency, because xraylib is a C library and it isn't available on pypi. However, since it is packaged on conda-forge, it should be easy to install in an anaconda/miniconda distribution.

@ericpre ericpre mentioned this pull request Oct 20, 2021
6 tasks
@ericpre ericpre removed this from the v1.7 milestone Oct 27, 2021
@ZanettaPM ZanettaPM mentioned this pull request Oct 8, 2022
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants