[JOSS Review] Initial installation + testing notes

[rdeits]

Hi @vavrines--I'm working on the review for JOSS; here are a few initial notes from the installation and testing process:

• The instructions here: https://xiaotianbai.com/Kinetic.jl/dev/install/ instruct users to add Kinetic but also build Kinetic and instantiate. Only that first instruction is actually necessary. Adding a package automatically builds it, and instantiate is only necessary when installing the packages listed in a Manifest.toml, which is not the case here. You can simplify those instructions to add Kinetic

• I tried following the instructions in paper.md, but got an error during the call to initialize. The full output is below:

julia> set, ctr, face, t = initialize("config.toml")
==============================================================
Kinetic.jl
Portable Kinetic Simulation and Scientific Machine Learning
==============================================================

[ Info: initializing solver

[ Info: Reading config from config.toml
--------------------------------------------------------------
matter = gas # material
case = cavity # case
space = 2d2f2v # phase
flux = kfvs # flux function
collision = bgk # intermolecular collision
nSpecies = 1 # number of species
interpOrder = 2 # interpolation order of accuracy
limiter = vanleer # limiter function
boundary = maxwell # boundary condition
cfl = 0.8 # CFL number
maxTime = 5.0 # maximal simulation time
x0 = 0.0 # starting point in x
x1 = 1.0 # ending point in x
nx = 45 # number of cells in x
y0 = 0.0 # starting point in y
y1 = 1.0 # ending point in y
ny = 45 # number of cells in y
pMeshType = uniform # mesh type
nxg = 0 # number of ghost cell in x
nyg = 0 # number of ghost cell in y
umin = -5.0 # starting point in u
umax = 5.0 # ending point in u
nu = 28 # number of cells in u
vmin = -5.0 # starting point in v
vmax = 5.0 # ending point in v
nv = 28 # number of cells in v
vMeshType = rectangle # mesh type
nug = 0 # number of ghost cell in u
nvg = 0 # number of ghost cell in v
knudsen = 0.075 # Knudsen number
mach = 0.0 # Mach number
prandtl = 1.0 # Prandtl number
inK = 1.0 # molecular inner degree of freedom
omega = 0.72 # viscosity index of hard-sphere gas
alphaRef = 1.0 # viscosity index of hard-sphere gas in reference state
omegaRef = 0.5 # viscosity index of hard-sphere gas ub reference state
uLid = 0.15 # U-velocity of moving wall
vLid = 0.0 # V-velocity of moving wall
tLid = 1.0 # temperature of wall
--------------------------------------------------------------

ERROR: MethodError: no method matching Setup(::SubString{String}, ::SubString{String}, ::SubString{String}, ::SubString{String}, ::SubString{String}, ::SubString{String}, ::SubString{String}, ::SubString{String}, ::SubString{String}, ::SubString{String}, ::SubString{String})
Closest candidates are:
  Setup(::S, ::S, ::S, ::S, ::S, ::I, ::I, ::S, ::S, ::E, ::F) where {S<:AbstractString, I<:Integer, E<:Real, F<:Real} at /home/rdeits/.julia/packages/KitBase/4GfNd/src/Type/struct_general.jl:23
Stacktrace:
 [1] set_setup(dict::Dict{Symbol, Any})
   @ KitBase ~/.julia/packages/KitBase/4GfNd/src/Solver/solver_set.jl:162
 [2] SolverSet(configfilename::String)
   @ KitBase ~/.julia/packages/KitBase/4GfNd/src/Solver/solver_set.jl:54
 [3] initialize(configfilename::String)
   @ KitBase ~/.julia/packages/KitBase/4GfNd/src/Solver/solver_init.jl:32
 [4] top-level scope
   @ REPL[5]:1

Note that I specifically installed v0.7.0, as specified in the review: openjournals/joss-reviews#3060

[vavrines]

Hi @rdeits

Thanks for the feedback!

• The documentation has been revised in the latest dev.
• I'm sorry for the inconvenience. The comments after # in the paper play as an explanation of parameters which are absent in general usage. I just realize that they are incompatible with the I/O. A quick fix is to remove # and the contents afterwards. The I/O method has been rewritten as well (vavrines/KitBase.jl@368a818) in the v0.4.3 of KitBase.jl. Now the issue should have been fixed.

[rdeits]

Ok, thank you. After updating to:

(Kinetic.jl_review) pkg> st
      Status `~/Projects/Kinetic.jl_review/Project.toml`
  [82403725] Kinetic v0.7.2
  [86eed249] KitBase v0.4.7

the initialize step succeeds, but the call to solve! fails:

t = solve!(set, ctr, face, t)

Output:

MethodError: no method matching solve!(::SolverSet{Setup{SubString{String}, Int64, Float64, Int64}, PSpace2D{Int64, Int64, OffsetArrays.OffsetMatrix{Float64, Matrix{Float64}}}, VSpace2D{Float64, Int64, OffsetArrays.OffsetMatrix{Float64, Matrix{Float64}}}, Gas{Float64, Int64, Int64, Int64, Float64, Float64, Int64, Float64, Float64, Float64, Int64}, IB2F{Vector{Float64}, OffsetArrays.OffsetMatrix{Float64, Matrix{Float64}}}, String}, ::OffsetArrays.OffsetMatrix{ControlVolume2D2F, Matrix{ControlVolume2D2F}}, ::Matrix{Interface2D2F}, ::Matrix{Interface2D2F})
Closest candidates are:
  solve!(::X, ::Y, ::Z, ::Z, ::Any) where {X<:AbstractSolverSet, Y<:(AbstractMatrix{var"#s317"} where var"#s317"<:AbstractControlVolume), Z<:(AbstractMatrix{var"#s294"} where var"#s294"<:AbstractInterface2D)} at /home/rdeits/.julia/packages/KitBase/o7fJb/src/Solver/solver_main.jl:93
  solve!(::X, ::Y, ::Z, ::Any) where {X<:AbstractSolverSet, Y<:(AbstractVector{var"#s317"} where var"#s317"<:Union{ControlVolume1D, ControlVolume1D1F, ControlVolume1D2F}), Z<:(AbstractVector{var"#s294"} where var"#s294"<:AbstractInterface1D)} at /home/rdeits/.julia/packages/KitBase/o7fJb/src/Solver/solver_main.jl:33

[vavrines]

@rdeits
I apologize for the inconvenience. I should have checked it again.
The example codes should be as follows

using Kinetic
set, ctr, xface, yface, t = initialize("config.toml")
t = solve!(set, ctr, xface, yface, t)
plot_contour(set, ctr)

where xface and yface record numerical fluxes along x and y directions respectively.
I've revised the paper, and the tutorials in the documentation (https://xiaotianbai.com/Kinetic.jl/dev/tutorial/) should all work fine.

[rdeits]

Thanks again for updating the examples--all of the code now appears to work exactly as expected.

I just have two requests before finishing the review checklist:

1. Could you please add the exact example you posted above to the Kinetic.jl/test/runtests.jl? (currently the initialize call is covered, but not solve! or plot_contour. I see that KitBase.jl and the other modules already have good test coverage, but having a complete example in the Kinetic.jl tests should reduce the likelihood of that example accidentally breaking in the future again.
2. There are a few minor errors in the paper--could you please take a look?

• Could you add one sentence to the summary about what the role of machine learning is here? For example, I could imagine using optimization or ML to try to design a cavity to achieve some fluid property. Or I could imagine using ML to try to approximate the entire fluid dynamics calculation to improve performance. Or perhaps something completely different.
• "KitBase.jl with basic physics" should be "KitBase.jl for basic physics", and likewise "KitML.jl for neural dynamics"
• I'm not sure what the intended meaning of "hooked" is in "neural equation can be hooked and solved". I would suggest "can be set up and solved" instead.
• "algorithmic efficiency for application" should be "algorithmic efficiency for applications"
• "are splitted" should be "are split" and "is splitted" should be "is split". I have no idea why the past tense of "split" is "split"--English is just kind of weird that way.
• "for broad devices" should probably be "for a broad range of devices"
• "Kinetic.jl focus" should be "Kinetic.jl focuses"
• "holds the perfect possibility" is a bit awkward. "is possible" might be better there.
• "It depends Flux.jl" should be "it uses Flux.jl" or "it depends on Flux.jl"
• "Closely coupling" should probably be "Close coupling" or "Easy integration"
• "we've develop" should be "we've developed" (or probably "we have developed" since contractions like "we've" are less common in formal writing like this)
• "from Python ecosystem" should be "from the Python ecosystem"

[vavrines]

Hi @rdeits

Thank you very much for the input!
The manuscript has been revised with the incorporation of all these suggestions, including

• The complete workflow is added into runtests.jl.
• A one-sentence summary of machine learning is added into the summary.
• The grammatical errors are fixed.

Please find the revised paper here (https://github.com/openjournals/joss-papers/blob/joss.03060/joss.03060/10.21105.joss.03060.pdf).

[rdeits]

Thanks for the updates--it all looks good to me!