JOSS review: Documentation

[tkoskela]

• I think the documentation should include instructions on how to reproduce the results. I've ran benchmarks.jl using PkgBenchmarks but it's not entirely clear to me how the results relate to what is reported in the documentation. The files perf.jl and perf.py probably should not be included in the package as they contain hard-coded file and path names that aren't part of the package.
• The paper comments on how good the interoperability is with other community packages. I'm curious why you have chosen to define your own physical constants, instead of using a community package, such as PhysicalConstants.jl
• The link to repo in

  Vlasiator.jl/docs/src/manual.md

  Line 233 in feda666

  More examples of customized plots can be found in the [repo](https://github.com/henry2004y/Vlasiator.jl/tree/master/src/examples).

  is broken

openjournals/joss-reviews#4906

[henry2004y]

Thanks for the review!

1. The link is fixed.
2. PhysicalConstants.jl is a nice collection of constants, but it come naturally with the dependencies on Unitful.jl which is something we haven't tested carefully. Having predefined units is good, but it unavoidably comes at a cost of performance. Also, in order to make computations convenient and consistent, we may have to define my own unit system for commonly used units in our community, which is also not fully tested. Thus we decided to define the constants inside the package (which is currently only ~ 10 lines). Currently based on this list, a few of the astrophysics packages depend on it. In the future, we may make a switch if PhysicalConstants.jl proves to be a nice addition.
3. About the benchmarks, first we want to make a clarification between the PkgBenchmark workflow and the comparisons between Julia and Python. benchmark.jl is used for tracking the development regressions and improvements within Vlasiator.jl: it is not used to show the comparisons between Vlasiator.jl and Analysator.jl. Showing the result from PkgBenchmark is not useful for users, but is beneficial for developers, so we exclude it from the document. As the reviewer pointed out, perf.jl and perf.py are the scripts used for generating the benchmarks shown in the document. The most tricky part, which is also mentioned in JOSS review: features #122 and JOSS review: documentation #115, is that VLSV format files generated from Vlasiator are either large or non-public. In order to show the comparisons at real work situations, our current solution is to run perf.jl and perf.py on private data on the server with different sizes. The test data for this package contains 3 VLSV files which are intently kept small. If we use those for the benchmarks, then only reading DCCRG grid variables at very small array sizes can be shown, which may not be ideal. Our current plan is to update perf.jl and perf.py and make better descriptions to the benchmark results. We would also like to hear about your opinions on how to handle the situation @tkoskela?

[tkoskela]

Apologies, I had not looked under Log for instructions on how to run the benchmarks. I think renaming it to FAQ makes it a bit more obvious, but I would also put a link to it from the Benchmarks page. Probably that's part of your plans in making better descriptions of the benchmark results.

I understand that the size of the data files may prevent you from distributing them as part of the package. I don't really have an universal solution for that, I would suggest looking at services your institute or HPC center may offer for data storage and distribution. I'm perhaps a bit confused about the issue of data being non-public. If there is no public data, it's difficult to see the benefits of an open source processing tool. I understand you might want to keep unpublished data private, but you should have something published that can be used to run the benchmarks on.

I would suggest at least clearly documenting the benchmarks that only make sense to run on large datasets that may not be available as part of the package, and providing data for the benchmarks where you can get meaningful results within the limits of what you can store on GitHub.

[henry2004y]

Thanks for the suggestions! Upon inquiring our colleagues, we now find some open-access Vlasiator data. We will update the benchmarks and add the links to the data.

[henry2004y]

In Vlasiator.jl v0.9.35, we have updated perf.jl, perf.py as well as the benchmark results. perf.jl and perf.py can be run directly to produce a major portion of the benchmarks listed in the doc. One of the files (file index 5) is private, while others are all publicly accessible.

We have also fixed some issues in comparing the performance between Python/Numpy and Julia, which should now give fairer results. Previous array reading benchmarks in Python have the sorting of cells in the data reading step; now it is moved to the metadata loading step for better comparison with the procedure in Julia.

[tkoskela]

I would still like to ask for a bit clearer documentation on Benchmarks. With some guesswork I can link some of the numbers reported in https://henry2004y.github.io/Vlasiator.jl/dev/benchmark/ to the output of benchmarkpkg(Vlasiator), for example file index 2 of "loading meta data" is probably "meta" => Trial(310.480 μs). If you could describe on the Benchmarks page which numbers I can expect to extract from running benchmarkpkg on the data you provide, and how, that would make me happy.

[tkoskela]

I would also note that I ran into an error

ERROR: LoadError: Cannot locate '(Julia)Artifacts.toml' file when attempting to use artifact 'testdata' in 'Main'

that seems to be reported in JuliaLang/IJulia.jl#1060. The solution in the first comment of the issue fixed it for me

[henry2004y]

I ran this issue as well and indeed the first comment fixed it. However, for some reason it didn't work for the pkgbenchmark that I added recently. In CI, my current workaround is to manually create a symbolic link under the benchmark folder:

Vlasiator.jl/.github/workflows/benchmark.yml

Line 26 in 19b203b

cd benchmark; ln -s ../Artifacts.toml Artifacts.toml

Then this works for both CI and local pkgbenchmark.

[henry2004y]

I would still like to ask for a bit clearer documentation on Benchmarks. With some guesswork I can link some of the numbers reported in https://henry2004y.github.io/Vlasiator.jl/dev/benchmark/ to the output of benchmarkpkg(Vlasiator), for example file index 2 of "loading meta data" is probably "meta" => Trial(310.480 μs). If you could describe on the Benchmarks page which numbers I can expect to extract from running benchmarkpkg on the data you provide, and how, that would make me happy.

Are you running perf.jl or benchmarks.jl @tkoskela ? In the benchmark page, the numbers are generated with perf.jl and perf.py, while benchmarks.jl is used for checking regressions within the pkg. I may cleanup the benchmarks further to get better links to each part.

Outputs of running perf.jl on my laptop:

julia> include("perf.jl")
[ Info: Benchmark file 1d_single.vlsv found...
[ Info: Benchmark file bulk.2d.vlsv found...
[ Info: Benchmark file 2d_double.vlsv found...
[ Info: Benchmark file 2d_AFC.vlsv found...
[ Info: Benchmark file 3d_EGI.vlsv found...
(1/3) benchmarking "plot"...
  (1/2) benchmarking "contour_nonuniform"...
  done (took 21.619722403 seconds)
  (2/2) benchmarking "contour_uniform"...
  done (took 22.029348411 seconds)
done (took 44.512377048 seconds)
(2/3) benchmarking "load"...
  (1/5) benchmarking "bulk.2d.vlsv"...
  done (took 1.100194611 seconds)
  (2/5) benchmarking "1d_single.vlsv"...
  done (took 0.831158252 seconds)
  (3/5) benchmarking "2d_double.vlsv"...
  done (took 1.328040379 seconds)
  (4/5) benchmarking "3d_EGI.vlsv"...
  done (took 21.270450767 seconds)
  (5/5) benchmarking "2d_AFC.vlsv"...
  done (took 21.220161608 seconds)
done (took 46.48213409 seconds)
(3/3) benchmarking "read"...
  (1/10) benchmarking "3d_EGI.vlsv_sorted"...
  done (took 20.983659774 seconds)
  (2/10) benchmarking "3d_EGI.vlsv_unsorted"...
  done (took 21.10458052 seconds)
  (3/10) benchmarking "bulk.2d.vlsv_sorted"...
  done (took 0.883095677 seconds)
  (4/10) benchmarking "2d_double.vlsv_sorted"...
  done (took 1.296831014 seconds)
  (5/10) benchmarking "2d_double.vlsv_unsorted"...
  done (took 1.426689205 seconds)
  (6/10) benchmarking "2d_AFC.vlsv_unsorted"...
  done (took 21.159254651 seconds)
  (7/10) benchmarking "2d_AFC.vlsv_sorted"...
  done (took 21.333452554 seconds)
  (8/10) benchmarking "1d_single.vlsv_unsorted"...
  done (took 0.80439711 seconds)
  (9/10) benchmarking "bulk.2d.vlsv_unsorted"...
  done (took 0.877172364 seconds)
  (10/10) benchmarking "1d_single.vlsv_sorted"...
  done (took 0.809524429 seconds)
done (took 91.409206892 seconds)
3-element BenchmarkTools.BenchmarkGroup:
  tags: []
  "plot" => 2-element BenchmarkTools.BenchmarkGroup:
          tags: ["2d", "3d"]
          "contour_nonuniform" => Trial(541.233 ms)
          "contour_uniform" => Trial(672.832 ms)
  "load" => 5-element BenchmarkTools.BenchmarkGroup:
          tags: ["1d_single.vlsv", "bulk.2d.vlsv", "2d_double.vlsv", "2d_AFC.vlsv", "3d_EGI.vlsv"]
          "bulk.2d.vlsv" => Trial(978.434 μs)
          "1d_single.vlsv" => Trial(318.396 μs)
          "2d_double.vlsv" => Trial(4.148 ms)
          "3d_EGI.vlsv" => Trial(642.755 ms)
          "2d_AFC.vlsv" => Trial(693.975 ms)
  "read" => 10-element BenchmarkTools.BenchmarkGroup:
          tags: ["1d_single.vlsv", "bulk.2d.vlsv", "2d_double.vlsv", "2d_AFC.vlsv", "3d_EGI.vlsv"]
          "1d_single.vlsv_sorted" => Trial(7.405 μs)
          "3d_EGI.vlsv_unsorted" => Trial(3.164 ms)
          "bulk.2d.vlsv_sorted" => Trial(31.172 μs)
          "2d_double.vlsv_sorted" => Trial(220.135 μs)
          "2d_double.vlsv_unsorted" => Trial(86.630 μs)
          "2d_AFC.vlsv_unsorted" => Trial(7.016 ms)
          "2d_AFC.vlsv_sorted" => Trial(35.849 ms)
          "1d_single.vlsv_unsorted" => Trial(6.866 μs)
          "bulk.2d.vlsv_unsorted" => Trial(16.736 μs)
          "3d_EGI.vlsv_sorted" => Trial(15.840 ms)
----------
 11.229765 seconds (885.92 k allocations: 6.243 GiB, 0.32% gc time, 3.78% compilation time)
----------
 70.401988 seconds (509.29 k allocations: 12.423 GiB, 7.21% gc time, 1.66% compilation time)

Outputs of running perf.py on my laptop:

➜  benchmark git:(master) ✗ python3 perf.py
Using LaTeX formatting
Using matplotlib version 3.5.1
Benchmark file 1d_single.vlsv found...
Benchmark file bulk.2d.vlsv found...
Benchmark file 2d_double.vlsv found...
Benchmark file 2d_AFC.vlsv found...
Benchmark file 3d_EGI.vlsv found...
1d_single.vlsv:
Loading metadata in 0.7706 ms
1d_single.vlsv:
Reading scalar DCCRG variable in 0.0583 ms
bulk.2d.vlsv:
Loading metadata in 1.0458 ms
bulk.2d.vlsv:
Reading scalar DCCRG variable in 0.1664 ms
2d_double.vlsv:
Loading metadata in 2.5013 ms
2d_double.vlsv:
Reading scalar DCCRG variable in 0.2801 ms
2d_AFC.vlsv:
Loading metadata in 298.1129 ms
2d_AFC.vlsv:
Reading scalar DCCRG variable in 33.9831 ms
3d_EGI.vlsv:
Loading metadata in 313.2423 ms
3d_EGI.vlsv:
Reading scalar DCCRG variable in 17.4951 ms
/home/hongyang/Vlasiator/analysator/pyPlots/plot_colormap.py:1306: UserWarning: Matplotlib is currently using agg, which is a non-GUI backend, so cannot show the figure.
  plt.show()
Uniform 2d plotting in 6.4596 s
/home/hongyang/Vlasiator/analysator/pyPlots/plot_colormap3dslice.py:1513: UserWarning: Matplotlib is currently using agg, which is a non-GUI backend, so cannot show the figure.
  plt.show()
AMR slice plotting in 4.6197 s
Reading FSGrid variable in 104.8189 s

[tkoskela]

I was running benchmarks.jl, thank you for pointing that out! I can verify the benchmark data, so I am happy to close the issue. Labeling the benchmark suites in perf.jl to match the documentation a bit better could be helpful.

[henry2004y]

Yeah this is indeed kind of confusing, because currently PkgBenchmarks requires the file benchmarks.jl so I cannot use it for comparing against Python with exactly the same tasks and larger datasets. But we will find a clearer (and automatic) way of doing this!