icbicket / spectrumimageanalysispy Goto Github PK

Looking at Goal statement 3:

Extract slices and areas from the dataset as desired by the user and communicated through the user interface, export these as desired by the user.

I noticed "desired by the user is used" twice. This makes me think that your goal statement could be broken down into two goal statements.

The first part being able to interact, extract, slice and manipulate the dataset.

The second part is the ability to the export the data in various file formats.

Disclaimer I am not familiar with this notation so maybe this is correct.

The way LaTex formatted the equation, it is unclear.

I assume the numerator is I(E).

I don't know what the denominator of the equation is. It appears to just be a sum sign. Was this supposed to be the sum of I(E(k)) for all values between k1 and k2?

This looks like a quick fix!

In 20.4.2, CreatMask() function (also other parts that have the same function and transition), what does the transition Creat mask mean ? I assume there should be some changes happening in some modules. I think maybe it is better to add more details here as well as the connection between state variable and transitions.

Marshall

Looking at the equation:

E = E(k)

I noticed f(k) is referenced in the description.
I assume this was supposed to be E = f(k)?

I saw some comments are present in the document, I think you need to remove those comments.

Please review my SRS document, @smiths ! I look forward to your feedback on improving the document.

I feel like your solution lends itself to another goal statement

A correct solution must be acceptable to a user experienced with EELS and/or CL data processing, as is relevant to the EELS/CL data being processed.

Is it the correct solution being described here, the visualization of the 3d spectra-microscopy created by the user?

Given that your goals are more about reading in, manipulating and exporting data, maybe this should also be an explicitly described goal?

I would definitely ask someone more familiar with preparing SRSs and software engineering principles.

I feel that the description of a correct solution isn't exactly applicable to your tool software because you made an exploratory tool that enables analysis. I don't know if it explicitly outputs a "solution" if you know what I mean.

Comments are available in 5c95557.

4. For Table 3 (Trace Between Anticipated Changes and Modules), there are different symbols which makes me confused. I suggest to create single module for Export, Data, etc.

Hi Isobel Claire,

Thank you for your MG document. I was glad to read, but I have a few modest issues to provide. All of them are subjective and just for references.

1.For M2-M5, if I’m reading it correctly, it seems that you can only use “SpectrumImageAnalysisPy” to implement the importing. So, I think it is enough to only to create “import module” to present the importing.

Please review the comments on your SRS. The comments are added in commit f6c8dea.

Sometimes in the spectrum imaging interface, the wrong spectrum is exported. Work-around is to make doubly sure that the right spectrum is active by toggling it on and off, but would be nice if this wasn't needed. Could happen when the active patch is switched without toggling the spectrum off and on.

Since you have a lot of modules and sections in MIS&MG. I think it is better to use crossing-reference for all modules mentioned. For example in Uses part of each section. It will be much more convenient for readers to track the connection between modules and clear about architecture of your program :). Anyway a great work done by you ! I don’t have more comments. Good luck with your final docs !

Marshall

Dear Isobel,

Seems you haven’t upload updated version for MIS.
I only have one issue for you currently.
As I see your MG. There are should be many level 3 Modules.
And actually you have sections for them in MIS. However why the table of module hierarchy doesn’t show level 3 Modules ?
Would you please fix this ?

I will provide more comments after you finish and upload it.
Thanks for understand ! :)

Best Regards,
Marshall

I do not know the source, but there is a TestPlan.lot file that comes up when git status is run. If it shouldn't be under version control, you should consider adding it to your .gitignore file.

Please review my test plan document, @smiths !

Please review my MG, @smiths !

I suggest adding a file, either a README or an INSTALL file, that tells users how to get started with your software. I believe that SpectrumImageAnalysisPy depends on libraries, like dm3_lib; you should let users know the dependencies and how they can resolve them.

None of this is necessary for the CAS 741 course, since I know there is an issue with sharing data, but going forward, installability will be a significant part of whether your software is used by others or not.

2. Based on your definition of M24, it seems that M24 is not using the Hardware Hiding Module. So I suggest to remove the arrow from M24 to M1.

Hi Isobel!

I read through your SRS! It was very well done. I found it easily digestible and I couldn't find any real flaws. Forgive the following issues if they sound nitpicky, I'm grasping at straws here lol

Cheers,
Keshav

In some your Import* modules, I see in Syntax section the name is Read* but in Semantics section, the name is Import*. Such as ReadDM3 and ImportDM3. I see input & output are exactly the same so I wonder are they the same function in you program ? If so, how about use the same name

Marshall

In section 5.1.2 File Export , in Image export and Image export - GUI the output is an image. So I did not understand how you are testing an image?

Old DM file import function uses dm3_lib, not sure if this external library is up to date with DM4. Choose an external library and update import functions to match the output.

Comments are available for your review in commit 7d62d03.

Contrast histogram has a giant peak at 0, thanks to the masked values, which are transparent in the image, but dominate the histogram. It would be nicer to eliminate this histogram peak at 0, to see the rest of the histogram better.

5. Based on the issue 3, does your software flow through in a linear way? I mean if some modules can be directed in the order such as 1D slice, 2D mask and 3D mask?

Base class and closeto functions are being deprecated in matplotlib - remove these from SpectrumPlotter.

Comments on your MG are available in commit f803626

Matplotlib 3.3.3 removed matplotlib.tracker.Base and SpectrumImagePlotter depends on this - need to remove this depedency.

I noticed with the lists, you have a tendency to end each statement with a semicolon (Section 4.1, 5.1.1., some of 5.21 etc)

I found this a bit distracting. I think this is habit from programming. I had the same thing in my first version of my Latex document. Maybe remove the semicolons for a cleaner look?

In section 5.1.1 Deconvolution error estimation you have explained that the difference between the deconvolved spectra and the original spectrum will be calculated and the RMS error will be plotted. But you did not tell whether it will be done manually or its an automated test.

Thank you for your remind Isobel ! I am not professional in background of your project. So if my comments are unprofessional then please do not consider and just correct me.

I would like to provide more comments from structure perspective. :). In MIS SI, Spectrum, Image are mentioned many times. I can see definition of Spectrum Image in SRS but I didn’t find Spectrum and Image. I wonder are they instances of classes defined by you ? How about add some comments about what is the difference between spectrum image, spectrum and image ?

Marshall

Please review my problem statement for the course!

Following error message received on using AlignLib.Align on six SEM images, after adjusting images and hitting Enter:
Traceback (most recent call last):
File "/home/isobel/.local/lib/python3.6/site-packages/matplotlib/cbook/init.py", line 216, in process
func(*args, **kwargs)
File "/home/isobel/Documents/McMaster/PythonCodes/DataAnalysis/src/AlignPlot.py", line 48, in MoveImg
self.ShowImages()
File "/home/isobel/Documents/McMaster/PythonCodes/DataAnalysis/src/AlignPlot.py", line 29, in ShowImages
self.fig.canvas.draw()
File "/home/isobel/.local/lib/python3.6/site-packages/matplotlib/backends/backend_tkagg.py", line 10, in draw
_backend_tk.blit(self._tkphoto, self.renderer._renderer, (0, 1, 2, 3))
File "/home/isobel/.local/lib/python3.6/site-packages/matplotlib/backends/_backend_tk.py", line 80, in blit
photoimage.blank()
File "/usr/lib/python3.6/tkinter/init.py", line 3548, in blank
self.tk.call(self.name, 'blank')
_tkinter.TclError: invalid command name "pyimage70"

In section 6.1 the input for SI .dm3 file and SI .h5 file is a file containing a fabricated spectrum but what is the case if the file is empty?

3. These seem like steps in a procedure ,but how each single function calls the next one? For example, how the results of Data Processing pass to Data Extraction? Is there a control module to direct the order each of these modules?

You might want to add *.bbl, *.blg, *.out and *.toc to your .gitignore file. These are all intermediate file created by LaTeX.

Saving pngs from the spectrumimageplotter removes trailing zeros from the energy range selected - fix this to keep the same number of decimals in all image names.

This theoretical model was well described.

I'm not too familiar with discrete and continuous mathematics, and I'm not sure if this relevant.

Would it be within the scope to show how the continuous model of the signal is converted to the discretized version? As in what assumptions were made?

I assume this is described in the reference by JM Zuo.

I just thought it would be insightful to see how the two equations in TM2 are mathematically linked.