MG-MIS slides review

[rnorouziani]

Dear Dr. Smith, The slides for tomorrow's presentation have been added to the project repository. I would greatly appreciate your feedback on it. @smiths

[smiths]

Quick feedback:

• I like the output module being separate. It might be simple now, but it could be more complicated in the future. The secret is the output format. By having the module separate you can also change where the output is sent. It could be sent to a file or to the screen.
• The magnetic core module has an "and" in the secret. That is a sign that you have more than one secret. You should scrutinize this. Having a separate module for physical constants often makes sense.
• The Helmholtz module is like a main module, or a control module? If it has magnetic calculation algorithms it seems like it might overlap with the the Magnetic Core Module.
• The uses relation is easier to read if the highest level modules are at the top of the page. It seems like the Helmholtz module should be at the top.
• For the uses relation it is nice if the modules are organized so that all of the arrows only need to point down
• Why doesn't the Helmholtz module use the input formal module?
• For the input format module, do you need a verify_param access program? Can you just rely on load_params to do the verification?
• For the input format module all of your inputs will come from the command line? This is fine if this is the use case your users want, but it can quickly get tedious to type inputs. Maybe a file for the inputs makes more sense?
• For the Output Verification Module, I1, I2, I3 are not types, rather than float, use real in the MIS
• Throughout the access program should show types, you mix variable names and types. If you want to name the parameters that is fine, but the type information is important to include (both for inputs and outputs)
• Magnetic core has state variables, but it doesn't seem clear how the access programs change the state variables
• For the CoilT module the constructor's name should be CoilT, the Output types should be CoilT
• The access programs for Helmholtz don't make it clear why all of the uses modules are needed. It is difficult to tell what Helmholtz is going to do.
• Magnetic core module has gradients as input. Where are these calculated?

It is difficult to see how some of the access programs will be used. I suggest that you sketch out some code that uses the access programs. This will help you visualize how the modules will be used.

[rnorouziani]

Thank you for your detailed feedback. I've attempted to incorporate your suggestions, and here is the updated version of my slides. I believe including a separate module for physical constants in my project could be unnecessary since I only have one physical constant, and by having it, the complexity increases. I modified the input module so that it reads the coil information from a file (since it remains the same throughout the runtime), and reads the target force and torque from the command line. I am not sure if this is the proper way to implement what I have in mind.

[smiths]

@rnorouziani this is an improvement. I do not have time to go through all of the details. I did notice the following:

• I'm assuming you are using "cons" for constructor. It is more consistent with most programming languages to use the name of the class as the constructor.
• Should the coils (in CoilT) be more than just the state of the parameters? It seems to me that calculate_B_diff only depends on the coil, so why not move this calculation to the CoilT. The calculate_magnetic_field_at_centre could also be moved to CoilT, with a boolean argument.