I didn't notice anything wrong with the performance calcs, but it's hard to say everything is right without comparing it to MATLAB (which requires me to resolve #25) and implementing snapshot testing.
The performance module was a little hard to follow with the different structs and passing around the engine as well as previously calculated performance things (eg, Heats needing a reference to a Powers to get the heat from pressure drops). But I recognize that the performance calculations are a bit awkward with how they all pull from different parts of the engine and components and other calculated metrics in order to get all the values needed. All the math seems sound, so I'm ok with the structure if it makes sense to you.
Originally posted by @dyreby in #47 (review)
I think two things are getting muddled together currently:
- Translating from an
Engine into data we need to perform our calculations.
- Actually doing the calculations.
I think doing a better job of logically separating those two things would go a long way to improving this code.
I didn't notice anything wrong with the performance calcs, but it's hard to say everything is right without comparing it to MATLAB (which requires me to resolve #25) and implementing snapshot testing.
The
performancemodule was a little hard to follow with the different structs and passing around theengineas well as previously calculated performance things (eg,Heatsneeding a reference to aPowersto get the heat from pressure drops). But I recognize that the performance calculations are a bit awkward with how they all pull from different parts of the engine and components and other calculated metrics in order to get all the values needed. All the math seems sound, so I'm ok with the structure if it makes sense to you.Originally posted by @dyreby in #47 (review)
I think two things are getting muddled together currently:
Engineinto data we need to perform our calculations.I think doing a better job of logically separating those two things would go a long way to improving this code.