Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix portability issues with wheel on manylinux1 platform #319

Closed
pramodk opened this issue May 4, 2020 · 5 comments · Fixed by #320
Closed

Fix portability issues with wheel on manylinux1 platform #319

pramodk opened this issue May 4, 2020 · 5 comments · Fixed by #320
Assignees
@pramodk
Labels
enhancement New feature or request

Comments

@pramodk
Copy link
Contributor

pramodk commented May 4, 2020

As we are targeting NMODL release with NEURON wheel, build the NMODL on manylinux1 platform and address any incompatibilities if any.
@pramodk pramodk added the enhancement New feature or request label May 4, 2020
@pramodk pramodk self-assigned this May 4, 2020
@pramodk pramodk changed the title Build wheel on manylinux1 platform Fix portability issues with wheel on manylinux1 platform May 4, 2020
pramodk added a commit that referenced this issue May 4, 2020
@pramodk
Fix incompatibility issues with manylinux1 platform
55397b2 
- manylinux1 platform provides gcc 4.8.1 for wider compatibility
- gcc 4.8.1 is first gnu compiler supporting `almost all` C++11
  except that it lacks const_iterator overload for std::vector::erase
  and std::vector::insert
- in this PR we introduce const_iter_cast thar turns const iterator
  into non-const-iterator with minimal changes to the code base
- change the compatibility to gcc 4.8.2 for cmake as well as json library
- change pybind11::literals::operator""_a to namespace pybind11::literals
  as documented in https://pybind11.readthedocs.io/en/stable/basics.html#keyword-arguments

fixes #319
@pramodk pramodk mentioned this issue May 5, 2020
Merged
@ohm314
Copy link
Contributor

ohm314 commented May 5, 2020 •
edited
Loading

As discussed on slack, we should also explore other docker platforms. Specifically scikit-build recommends using dockcross-manylinux
https://scikit-build.readthedocs.io/en/latest/generators.html?highlight=dockcross-manylinux#linux
Alternatively you mention the manylinux-2010 platform. Can you explain why exactly manylinux1 is needed for nmodl?

The different options should be evaluated for nmodl before a decision is made. IMHO, since nmodl should not be shipped in the neuron wheel it doesn't necessarily need to be built with manylinux1, or is there another requirement that forces manylinux1?

@pramodk
Copy link
Contributor Author

pramodk commented May 5, 2020

Yes. But note that dockcross-manylinux or multibuild are services and underlying they uses same manylinux platforms. So portability issues for the code needs to be handled independently.

@mgeplf
Copy link
Collaborator

mgeplf commented May 5, 2020

Alternatively you mention the manylinux-2010 platform. Can you explain why exactly manylinux1 is needed for nmodl?

I second this: I would really consider doing manylinux2010 - you get the newer compiler, so you don't have to do as many work-arounds, and I'm not sure there are many systems out there that are running python 3.5 (~2015 - a reasonable minimal python version, IMO) that don't have support for manylinux2010 (ie: CentOS 6, [similar to RHEL6, circa 2010/2011)

@pramodk
Copy link
Contributor Author

pramodk commented May 5, 2020 •
edited
Loading

I second this: I would really consider doing manylinux2010 - you get the newer compiler, so you don't have to do as many work-arounds,

@mgeplf : I am glad that you looked at this discussion already! Actually, I had so little understanding of python wheels and what it means the portability. During last week I started reading about this topic and summarising all references in neuronsimulator/nrn#511. My intent was to understand this topic bit more and then have a discussion with you and @wizmer about this . For example, why you guys still building manylinux1 wheels and choice of C++ standard (meeting notes).

Can you explain why exactly manylinux1 is needed for nmodl?

Now about this : the reason I started looking at this is that for NEURON we setup manylinux1 wheels in neuronsimulator/nrn#468. I started building coreneuron and NMODL support in NEURON wheel and that's where I created this issue. Note that Neuron+NMODL+CoreNEURON are <=C++11 today so manylinux1 support is "almost free".

@mgeplf
Copy link
Collaborator

mgeplf commented May 5, 2020

I moved libsonata to manylinux2010:
https://github.com/BlueBrain/libsonata-wheels/blob/master/.travis.yml#L9

I suppose MorphIO switchover hasn't changed yet due to not having time - I have a branch there, and tested it, but I need to update it, and sync it w/ how the deployment in libsonata works for using testing.pypi.org.

pramodk added a commit that referenced this issue May 6, 2020
@pramodk
Fix incompatibility issues with manylinux1 platform (#320)
eb57b10 
* Fix incompatibility issues with manylinux1 platform
- manylinux1 platform provides gcc 4.8.1 for wider compatibility
- gcc 4.8.1 is first gnu compiler supporting `almost all` C++11
  except that it lacks const_iterator overload for std::vector::erase
  and std::vector::insert
- in this PR we introduce const_iter_cast thar turns const iterator
  into non-const-iterator with minimal changes to the code base
- change the compatibility to gcc 4.8.2 for cmake as well as json library
- change pybind11::literals::operator""_a to namespace pybind11::literals
  as documented in https://pybind11.readthedocs.io/en/stable/basics.html#keyword-arguments

fixes #319

Co-authored-by: Omar Awile <omar.awile@epfl.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pramodk pramodk

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix incompatibility issues with manylinux1 platform BlueBrain/nmodl
3 participants
@ohm314 @pramodk @mgeplf