Review on module guide

[JovieL25]

MG is available for review. The main changes include:

1. Section 4.1 Anticipated Changes
2. Section 5 Module Hierarchy
3. Section 7 Module decomposition
4. Section 7.2 Behaviour-Hiding Module
5. Section 7.3 Software Decision Module
6. Section 8 Traceability Matrix

[JovieL25]

Hello, @smiths,

I have updated MG by adding a link to the specific module or function in the design document. Let me know if this would make things more straightforward.

[smiths]

Hello @JovieL25. My feedback is as follows:

• page ii: the F and the R in "Non-functional requirement" should be capitalized
• page ii: the g and r in "Aorta geometry reconstruction" should be capitalized
• page 2, AC4: "the input parameters required to execute the algorithm" is not a good anticipated change. An anticipated change is something you can modify without changing the interfaces of your classes. If the input parameters change the classes will have to be modified. You could say instead that the secret is the "data structure to store the input parameters."
• page 2, AC5: "The methods to get the inputs from the user" is the same secret as AC2. You could change AC2 to the AC5 text, if you like the wording better.
• page 2, AC8: "The program state is likely changing during the processing". This is true of all software. You don't need this AC. It also isn't an AC that would map to a specific module - it would map to almost all the modules. Based on your mapping to M4, I think what you are after here is something like "How the overall control of the calculations is orchestrated."
• page 2, AC9: You don't want data type to be a likely change. You want to design data types that support your likely changes. For this one, I suggest: "The format of the final output data." (Format is a generic term. The format can be to a file or to the screen.)
• page 2, UC2: Do you mean that all volumes will have the same dimension? This seems like an unlikely unlikely change to me. Won't different images have different dimensions?
• page 3 - do you want to remove M10 given your new algorithm?
• page 3 - don't make your module SITK - this isn't abstract. It should be possible for someone to use another library to provide these services. You should instead characterize the services provided by SITK. You can say that the module is implemented by SITK, but you shouldn't name it SITK. :smile:
• page 3 - same point about numpy - characterize what numpy does, and say it is implemented by numpy, but leave it abstract so that a different numerical library could be used instead.
• page 6 - We should remove 7.2.8
• page 6 - Do you need axial segmentation (M9) separate from Aorta Segmentation (M8)? I think with your new algorithm you just need M8?
• page 6 - if you keep "Axial Segmentation" you should move it to a Behaviour Hiding module
• Table 3 - AC9 should map to something
• Page 9 - Provide a use hierarchy

I'd like to see the mapping between your modules and your code. Can you make a table that shows for each module the "piece" of code that is responsible for implementing it? The "piece" can be a class, or a piece of code inside a file.

[JovieL25]

As discussed on issue https://github.com/smiths/aorta/issues/2,

I am still working on the use hierarchy, but the rest of the MG can be re-reviewed 👍.

[JovieL25]

I am not sure if this Use Hierarchy is correct, let me know your thought @smiths .

[smiths]

The hierarchy looks like it is on the right track. However, shouldn't M2 use M3, since M2 would likely be responsible for populating M3 with values?

Does M8 really use M7? I thought that the crop volume was independent of the segmentation algorithm. You could do the crop volume and never proceed to the segmentation? Is the crop volume module actually the image?

[JovieL25]

As discussed in the meeting, I will add a new M11 Digital Enhancement Module, and change the use hierarchy.

The use hierarchy will look similar to the image below:

This issue is closed for now.

[smiths]

Looks good, except for the line starting at the lower left corner of M4. I think that line can safely be deleted.

[JovieL25]

Hi @smiths, Here is the new use hierarchy, let me know your thoughts.

[smiths]

Looks good, expect I'm confused about M5. There is an arrow pointing up to M5 and there is an arrow pointing down from M5. I agree that M4 uses M5. What other modules do you intend to have use M5? I don't feel like M5 should use any of other modules, but maybe you mean for it to do so?

[JovieL25]

Hello Dr. @smiths,

The arrow pointing from M5 to M2 is a mistake. I will change it and the rest of the use hierarchy should be good.