DeGoede review #1
Conversation
| - MATLAB (tested in R2023b) | ||
| - Symbolic math toolbox | ||
| - RF toolbox (for smith plots) | ||
| - the paretoFront function can be found [here](https://github.com/symbiotic-engineering/MDOcean/blob/main/mdocean/optimization/multiobjective/paretoFront.m) |
There was a problem hiding this comment.
maybe sub-module for the pareto front function? We are an optimization lab, so maybe it fits well into the SEA Lab utils?
There was a problem hiding this comment.
I think linking the script that generates fig 6 is ok becasue it is a standalone thing. but the need to go grab a script from another repo just to get the code in this repo to work is a bit inconvenient
There was a problem hiding this comment.
maybe clean out some of the commented out code, I'm assuming it's outdated. If you think it might be important later then maybe a separate branch?
There was a problem hiding this comment.
Would be good to add some comments. Point out equation numbers in paper that line up with stuff in the code. And add comments that mention where other equations/combinations of equations come from.
There was a problem hiding this comment.
make it clearer what code goes with each figure, a function that generates each figure would be nice, if you want to have one file then run all the functions that could be a nice way to have obvious separations while also making it easy for someone to recreate the figures.
There was a problem hiding this comment.
This is also in need of a lot more comments, explain what each step of the derivation means, explain what variables like wt_s and bi are.
There was a problem hiding this comment.
also if you are going to leave the other plots in there, I think you should have some comments on what they mean, if it's not worth writing comments on then they probably don't really need to be in the code
There was a problem hiding this comment.
once again, more comments would be nice, what is e1, e2, and e3, where are these equations coming from?
There was a problem hiding this comment.
Error: Unrecognized function or variable 'improvePlot'.
from line 25, perhaps need sea-lab-utils?
There was a problem hiding this comment.
I can guess as to what this does, but it would be nice to include in readme if necessary, but it also seems to me like maybe this is just a personal utility script and doesn't need to be in the repo?
|
overall, this repo could benefit from a lot more comments as it was hard to follow the code. Explaining variable names, identifying where equations are coming from/why they are being used, and pointing out equations numbers and figure numbers when they are created could go a long way. |
Scrutineering