Io refactor

[marcelo-alvarez]

(this pull request is identical to and meant to accompany desihub/desispec#1082)

A major change to how specex is called by desispec, using pybind11.

Calculations performed on the same day using (i.e. with specex.py as called by desi_compute_psf_mpi) https://github.com/desihub/desispec/tree/spx_io_refactor and (with directions for compiling the python shared object module in subdirectory specex_pybind) https://github.com/desihub/specex/tree/io_refactor should produce an identical merged psf fit file to that using https://github.com/desihub/desispec and https://github.com/desihub/specex

Please note this pull request is additionally dependent on branch spx_io_refactor of https://github.com/desihub/desispec, as reflected in the URLs above.

[marcelo-alvarez]

Using the gcc compiler, the io_refactor branches of desispec and specex now produce identical output to that from the master branches, on cori. Additionally, the documentation and source directory structure have been revised to be consistent and remove extraneous files. The pull request is now ready for review.

[julienguy]

Successfully tested along with desispec branch spx_io_refactor. Results as same PSF as Mt Blanc for the same inputs. Ready to merge for me.

[sbailey]

It appears that this branch brought in all of pybind11 itself as a subdirectory of specex. Was that intended and/or necessary or is it a leftover of an accidental git add? Could this be structured like fiberassign which uses pybind11 without including pybind11 itself in the package source?

[marcelo-alvarez]

It appears that this branch brought in all of pybind11 itself as a subdirectory of specex. Was that intended and/or necessary or is it a leftover of an accidental git add? Could this be structured like fiberassign which uses pybind11 without including pybind11 itself in the package source?

I think pybind11 is included in fiberassign, see https://github.com/desihub/fiberassign/tree/master/src/pybind11

I will look into it. It needs to exist somewhere, but not once for every repo that uses it, necessarily.

[tskisner]

@sbailey , including the minimal pybind11 headers in a project is standard practice. This way each project can track a particular version internally. If you were to put pybind11 into some other place that was globally accessible, then it would force all packages to use that version. It would also mean that this location would have to be available when running "python setup.py build" on any system. It is much better to bundle the headers (not the full package with docs, tests, etc) with the code that is using them.

[marcelo-alvarez]

@sbailey , including the minimal pybind11 headers in a project is standard practice. This way each project can track a particular version internally. If you were to put pybind11 into some other place that was globally accessible, then it would force all packages to use that version. It would also mean that this location would have to be available when running "python setup.py build" on any system. It is much better to bundle the headers (not the full package with docs, tests, etc) with the code that is using them.

Indeed, since it's a header-only library, the overhead is quite small and it makes it more self-contained and portable.

[julienguy]

One remark. Please make sure to have everything in place for the DESI environment installation (compilation, correct PYTHONPATH .. etc), because it will have to work as soon as we merge this PR.

[sbailey]

pybind11: fiberassign has 26 pybind11 files (564K) , while this branch of specex has 202 pybind11 files (2.2M) which made me think we were bringing in far more of pybind11 than we minimally have to. i.e. don't we only need pybind11/include/pybind11/ ? But maybe that's a topic for a future cleanup?

[tskisner]

Ah, yes, you should delete the tests, docs, tools directories and just put the headers into the pybind11 directory. Maybe put a README somewhere specifying the pybind11 version that is included in there, so that in the future people can more easily see what upstream version was used and whether they should be updated.

[marcelo-alvarez]

pybind11: fiberassign has 26 pybind11 files (564K) , while this branch of specex has 202 pybind11 files (2.2M) which made me think we were bringing in far more of pybind11 than we minimally have to. i.e. don't we only need pybind11/include/pybind11/ ? But maybe that's a topic for a future cleanup?

This can easily be cleaned up now.

[marcelo-alvarez]

I've merged my installation modifications into spx_io_refactor and io_refactor branches of desispec and specex. A local installation on nersc in the cloned directory of io_refactor branch of specex following

  export CC=gcc; export CXX=gcc
  python setup.py install --prefix ./code 
  export PYTHONPATH=$PWD/code/lib/python3.8/site-packages/libspecex-unknown-py3.8-linux-x86_64.egg:$PYTHONPATH

and then running desi_compute_psf with the spx_io_refactor branch of desispec should give identical results to running desi_compute_psf with the master branches.

[sbailey]

Merged by hand due to merge conflicts with master. Onwards to re-testing master branch!