JOSS Review #2
Description
This review follows the submission at openjournals/joss-reviews#8592.
Submitted article
Section "An Example": it's hard to decipher exactly what the data represents with the little information that is given when you do not know the problem beforehand. Giving the numbers of sources and destinations in the text would already help. For instance, on line 55:
between the three sources and four target
Design
In the whole package, you make a quite clear distinction between problem data and algorithm parameters. In the future, I think it'd be better to prepare your package for several solving techniques for the same problem: for instance, solve a TSP using (B)RKGA or MIPs.
Documentation
I do not see documentation for types such as ShortestPathProblem. In practice, that means that just reading https://jbytecode.github.io/OperationsResearchModels.jl/dev/network/#OperationsResearchModels.solve-Tuple{ShortestPathProblem} gives you no clue about how to use the solver. The same problem appears with Connection: what are the three arguments supposed to be? Is the graph supposed to be directed or not? How do you set the source and destination nodes? @kwdef is a partial solution to these problems, you could write Connection(from=..., to=..., value=...), but it doesn't explain what the value is. You sometimes have proper documentation for smaller types (such as https://jbytecode.github.io/OperationsResearchModels.jl/dev/project/#OperationsResearchModels.CPM.PertActivity), but all of them would benefit from some docs.
Given a matrix of distances, returns a TravelinSalesmenResult with the best route and its cost.
Missing g in TravelinSalesmenResult. (In text and code, actually.)
https://jbytecode.github.io/OperationsResearchModels.jl/dev/simplex/ : the docstring format is quite different from the other parts of the documentation, and Documenter doesn't recognise it. It considers your Python sections "Description" and "Returns" as code blocks, which prevents any line wrap, for instance.
You never really document the way you are solving problems: to get that information, you have to peek at the code. It's fine if you are using an off-the-shelf solver, less so for educational purposes.
Code
I have a hard time understanding the heuristics in https://github.com/jbytecode/OperationsResearchModels.jl/blob/36fa94ba7923ec8a4c6f71436a9e3e75e70b5216/src/network.jl to determine the source and destination nodes. I understand your code as: if a node has no incoming edge, it's the source; if it has no outgoing edge, it's the destination. That means that you have to tweak your graph for each and every call, removing parts of your graph to respect these conditions.
Still in the network module, you create varsym. The creation causes problems (unicity, mainly: you never document that you only support graphs and not multigraphs), but most importantly: you don't use it anywhere and I'm not sure it's useful for users to see them.
Connections are allowed to have Any values, but you feed them into a MIP model. The error messages will be subpar if users give something unrelated, say a string or a structure. The design in itself is fine for a a package that targets education; you could just add a loop to check whether all your Connections have a type that matches your expectations (i.e. floats).
Given that you target students, still for shortest paths, the names leftexpressions and rightexpressions are meaningless: inflow and outflow would make more sense in this case.
Why are you using completely different representations for shortest paths and TSPs? I think you should also define a TSP structure to hold the data for the problem for consistency purposes.
In parts of your simplex module (Gauss-Jordan), you use @fastmath. Are you extremely sure that this is a good idea? The major problem is that you have zero accuracy guarantees with that macro (see https://gcc.gnu.org/wiki/FloatingPointMath, for instance): floating-point arithmetic is not associative, but this macro is based on this assumption.