SWIFTSIM / swiftsimio

Python library for reading SWIFT data. Uses unyt and h5py.
GNU Lesser General Public License v3.0
18 stars 13 forks source link

Add API functionality #176

Closed harryjmoss closed 1 year ago

harryjmoss commented 1 year ago

As part of work between UCL and Durham, we have produced a prototype FastAPI app for sending numpy arrays derived from HDF5 files, as well as other SWIFT objects.

Here is the API

kyleaoman commented 1 year ago

@JBorrow the time allocated to the team at UCL to work on this has now ended so they're handing what they have (which is now fairly mature) over to us. I think that what we should do from here is:

  1. Merge this PR into a feature branch to wrap up some loose ends before merging to master.
  2. In that feature branch move the API code into a separate module (or submodule?) of swiftsimio. I think it makes sense to distribute it in the swiftsimio package.
  3. After that there's a little bit of documentation work to do in the swiftsimio narrative docs. At a minimum we should point to the remote API docs as an available feature, and probably add a new page with some cookbook examples. This would be about the earliest to consider merging into master?
  4. Then there's a bit of work to do to set up a persistent server on cosma (on dataweb, I think). The RSEs have left some suggestions of plausible ways this could work.
  5. A little bit of development needed to decide how we want to offer available simulation data through the server. Loosely the server would maintain a dictionary mapping labels to hdf5 files on disc, something like {"flamingos_NxxxxLxxxx_physicslabel_snapnum": "/cosma8/.../snap.hdf5"}, and then a user should be able to set up a RemoteSWIFTDataset with a label plus their credentials.
  6. Set up a webpage (on/linked from the VirgoDB pages?) pointing to the relevant documentation and tabulating the data available from the server by label. Maybe we can set everything up with a test dataset and then roll out an initial simulation dataset that we can announce more widely. Perhaps Flamingos makes a good candidate?

If you agree with step 1, at least, then please go ahead and approve the merge to a branch and we can work from there :)

kyleaoman commented 1 year ago

@MatthieuSchaller you likely want to be aware of this PR, maybe you have input on the proposed workflow from here.

kyleaoman commented 1 year ago

Actually there's no reason not to merge this into a development branch, and I need to make a few commits to fix bugs due to the new SWIFTUnits._handle attribute - and don't have push permission on the fork. @JBorrow detailed review can wait for the eventual PR to master, but perhaps worth having a browse of the branch in the meantime.

kyleaoman commented 1 year ago

Thanks @JBorrow. It's on my list to make a draft PR for this branch with a to-do list before it can be merged. Getting testing set up alone is going to be a big job. Ideally a lot of existing tests should be refactored to use fixtures so that we can test both the stand-alone and "client" SWIFTDatasets. Always hesitant to start making significant changes to tests, though, so will want to brainstorm that a bit before investing any effort. Figuring out how to host a server for the tests is also a bit of a tricky one - probably possible but not similar to anything I've tried before. Lots of other little stuff to do, too.