Closed archermarx closed 4 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 84.11%. Comparing base (
9b9247a
) to head (1e2619b
).
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
I'm gonna mark this ready for review. @DecBrick, could you take a look?
Looks good so far aside from the comments below.
For future steps, with regards to the params structure, it may be good to have a method to be able to call all of its keys in a concise manner from a solution object to allow the user to be able to see what properties are saved. A native way to do this in Julia may exist, but I haven't encountered it so far.
i think fieldnames(typeof(sol.savevals[1]))
would work, but i could add a saved_properties
method that does that for you
Well it does, maybe not do a full method but make a note about that in the documentation as the documentation on what fields the object has may not always be up to date.
I've added a HallThruster.saved_fields()
function which will list all fields that get saved to sol.savevals
. I've also updated the jupyter notebook to call this instead of using the static list of fields, and the docs so that this field is mentioned.
Addresses #121 in part, as well as making some other changes to the internals. The main goal of this is to remove the vestiges of DifferentialEquations.jl. Previously, we had used that library for timestepping, and the entire design of the code was based on that. However, that led to some awkward design choices. Now that we no longer use DifferentialEquations, I would like to restructure the code in a way that makes more sense. This PR is a draft for now.
What's changed
U
. This saves about 3% of the computational time as we have reduced the number of rows we iterate through by 1Plots
and others) so that the only heavy test dep isSymbolics
, which we use for OVS.ODEProblem
thing and associated structureStill to come (in later PRs)
params
andcache
. These were designed to work with the DifferentialEquations.jl interface, but it's difficult to see what's in there. Ideally, we'd replace with more descriptively-named structures (perhaps "simulation" and "data"?). This would be a pretty large breaking change, so perhaps we could include methods for accessing things the "old way" for a while