Stem eds demo #7
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
|
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:34Z Define IMP jorgenasorhaug commented on 2024-06-12T06:59:53Z It has been defined in section 1. (plural, IMPs) |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:34Z Is it necessary for this tutorials? jorgenasorhaug commented on 2024-06-12T07:04:20Z No. The intention is to show a possible workflow so that the path can be set to where the dataset is saved. |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:35Z Same as above |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:36Z In the interest of focusing on the content and reducing clutter in the notebook, I would remove all occurrence of
The data are necessary to be able to run the notebook. If they are not already on zenodo, it would be good to upload the data there, so that the pooch library can be used to download them easily. jorgenasorhaug commented on 2024-06-12T07:49:10Z The data is already on zenodo as stated in the introduction. I can add the step with pooch.
The files are located in a zipped folder. I am struggling with unzipping using the notebook without corrupting the files. Hope it's ok I just write that the folder must be manually unzipped (unless there's a workaround?)
The files are locataed in a subdirectory. Should I add a couple of lines to get the directory automatically, or should I write it as a string? |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:37Z Is there anything in particular that you want to check in the metadata? If so, specify, otherwise better to remove it. jorgenasorhaug commented on 2024-06-12T07:53:08Z My intention is to show how a workflow of STEM-EDS data analysis can be, which might involve checking the data, metadata, data type, etc. If this is of no interest, the notebook can indeed become shorter by focusing more on the objective. |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:38Z Explain what it the relevant information here? Why do you need to check the data type? |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:38Z Maybe explain why it would not influence the final results or provide a reference to support this statement? jorgenasorhaug commented on 2024-06-12T07:54:52Z I agree
|
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:39Z Clean up the comment: include or remove jorgenasorhaug commented on 2024-06-12T10:10:38Z My mistake.
The intention here is to let the user inspect the data and see that there are some additional peaks in the spectra that are not covered by the defined elements expected in the specimen. Hence the following lines. If this is redundant, I can write the notebook to be more focused on the end objective. |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:40Z Line #2. s_sum.add_lines() Is it necessary? jorgenasorhaug commented on 2024-06-12T10:18:25Z No :)
|
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:41Z In the interest of removing possible clutter in the notebook, I would set the minimum requirement to hyperspy 2.0 and exspy instead of using a try expect block here:
Simplify to:
jorgenasorhaug commented on 2024-06-12T10:19:13Z I agree
|
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:42Z Explain what you are inspecting. |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:42Z The materials database is not part of the exspy API reference, so I would mentioned here. If you think that this functionality is useful, then there should be a public API for this. For now, this should be removed as this should be removed, as this is not good practice to use non-public API. |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:43Z What does jorgenasorhaug commented on 2024-06-12T10:26:42Z signal binned and cropped. Will do |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:44Z This is not necessary at this stage - maybe at the beginning? You already have been using some of the EDS functionality (plotting EDS line). |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:45Z What is it used for? |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:46Z Maybe explain the reasoning to save the sum spectrum or remove it? Give a more explicit name? |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:46Z Is it necessary (this cell and the next two cells?) to check, since it has been done before? |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:47Z Explain why you disable the background in the model? The default is a polynomial of order 6 and later you add a polynomial of order 10. Add comment on why order 10 is better than 6? jorgenasorhaug commented on 2024-06-12T10:33:08Z The intention was to stay consistent with my published work (as written in the intro). When I did the analysis (older hyperspy version), I found that the sum spectra of the model and the raw data became more similar when using an order 10 for the background. For this example, I guess it doesn't matter that much. If preferable, I can use order 6? |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:48Z Explain briefly what is to be seen in the components and the values? |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:49Z This is a bit misleading: jorgenasorhaug commented on 2024-06-12T10:34:41Z My bad
|
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:50Z Can you comment on the impact of the calibration to the results here? Does it improve the results? jorgenasorhaug commented on 2024-06-12T10:45:05Z Yes |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:50Z Line #1. m.multifit(iterpath = 'serpentine', show_progressbar=True) In the interest of removing as much clutter as possible, I would the optional parameters, which are the default values (otherwise set in the preferences by the user).
jorgenasorhaug commented on 2024-06-12T10:45:51Z Will do |
|
View / edit / reply to this conversation on ReviewNB ericpre commented on 2024-06-11T20:00:51Z Can you be more specific with what needs to be checked? jorgenasorhaug commented on 2024-06-12T10:46:20Z Yes
|
There was a problem hiding this comment.
@jorgenasorhaug, thank you for the pull request. You can more easily read the review comments at https://app.reviewnb.com/hyperspy/exspy-demos/pull/7/.
The notebook needs to be easier to read, so I suggested many small simplification.
|
It has been defined in ection 1. (plural, IMPs)
View entire conversation on ReviewNB |
|
No. The intention is to show a possible workflow so that the path can be set to where the dataset is saved. View entire conversation on ReviewNB |
|
The data is already on zenodo as stated in the introduction. I can add the step with pooch View entire conversation on ReviewNB |
|
My intention is to show how a workflow of STEM-EDS data analysis can be, which might involve checking the data, metadata, etc. If this is of no interest, the notebook can indeed become shorter.
View entire conversation on ReviewNB |
|
I agree
View entire conversation on ReviewNB |
|
My mistake.
The intention here is to let the user inspect the data and see that there are some additional peaks in the spectra that are not covered by the defined elements expected in the specimen. Hence the following lines. If this is redundant, I can write the notebook to be more focused on the end objective. View entire conversation on ReviewNB |
|
No :)
View entire conversation on ReviewNB |
|
I agree
View entire conversation on ReviewNB |
|
signal binned and cropped. Will do View entire conversation on ReviewNB |
|
The intention was to stay consistent with my published work (as written in the intro). When I did the analysis (older hyperspy version), I found that the sum spectra of the model and the raw data became more similar when using an order 10 for the background. For this example, I guess it doesn't matter that much. If preferable, I can use order 6? View entire conversation on ReviewNB |
|
My bad
View entire conversation on ReviewNB |
|
This step is done in the user guide. I assumed it was necessary?
View entire conversation on ReviewNB |
|
Will do View entire conversation on ReviewNB |
|
Yes
View entire conversation on ReviewNB |
|
My bad. I had issues with copying the metadata, and didn't want to rerun the first line. I'll fix it. View entire conversation on ReviewNB |
|
Not sure if I am allowed to.. The values are from a software installed on the microscope. View entire conversation on ReviewNB |
|
I agree View entire conversation on ReviewNB |
|
Not necessary, but more convenient. I can write a list and comment that the orders between the k-factors and the elements should match? View entire conversation on ReviewNB |
|
I agree View entire conversation on ReviewNB |
|
Can do :) View entire conversation on ReviewNB |
|
My bad View entire conversation on ReviewNB |
|
Can do :)
View entire conversation on ReviewNB |
|
My intention with this was to make sure that the "spatial distribution" of the signal wouldn't influence the clustering. If you think it would be useful to show both approaches, I can add that too. View entire conversation on ReviewNB |
|
I agree View entire conversation on ReviewNB |
|
As I wrote in the introduction, I wanted to investigate what alloying elements might be found in the layers. I can highlight this in a new cell above? Further, the intention with using cluster analysis is the simplicity in the approach.
Not sure I understand what you mean? The two spectra are the summed raw signal from the respective layers? View entire conversation on ReviewNB |
|
Sure :) View entire conversation on ReviewNB |
Provided a STEM-EDS demo where element quantification and signal clustering (cluster analysis) is performer.