Closed jwreep closed 2 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 70.27%. Comparing base (
3ed98ff
) to head (bb05eb4
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@wtbarnes, please review.
I'll have a think about the "brittleness" aspect of it, so don't merge it yet. It's true that hard coding the indices is not great, particularly if the trm
files were ever updated in HYDRAD. (I suppose it's not likely to happen, but I have in the past wondered if it would be helpful to split background heating apart into its own term.)
pandas.read_csv
might be able to be used here, similarly to how the .phy
files have np.loadtxt
to parse them. You can skip rows with the pandas
function, so it could be used to create four separate dataframes. Cleaner and likely more robust code, but I suspect it would be slower because it would have to be read the file four times essentially. There might be a clever way to read it once and split into four dataframes, though . . .
Right, all indices have been removed. This isn't perfectly robust to changes in HYDRAD, but all that would need to be done is to add any new items to the lists of terms if there's a change in any of the equations.
This probably isn't the most efficient since it reads the files multiple times, though.
This might need a new unit test. I just pushed a fix that would not have given the correct result. Perhaps checking that one of the columns is not zero everywhere?
@wtbarnes This one is ready for review. The changes should be more robust.
Great! This looks good to me. The lack of repetition between the correspondence of the indices and variable names should make this more robust to changes in the layout. Thanks for all of your effort on this!
Addresses #168
I've added a full parsing of the .trm files. Hopefully the naming scheme is reasonable, but happy to edit that. I've also changed the variable names for the conduction variables in the .phy files, which are more accurately heat fluxes.