Re: [OpenBabel-Devel] maeparser depends on boost iostreams, can openbabel use it?

2019-06-11 Thread Patrick Lorton
Thanks all for the lively conversation.  I've updated the pull request to
check for boost and only build maeparser if it's available (printing to the
user saying it's skipping maeparser if it's not available).  I'll probably
ping a few package maintainers about making sure to add a boost requirement
when/if this is eventually released.

Pat

On Tue, Jun 11, 2019 at 1:39 PM Koes, David  wrote:

> I agree with Geoff for all the reasons listed. Not being able to use boost
> is a constant annoyance.  Making any boost dependent features conditional
> on boost available in cmake seems like a great compromise that will provide
> the feature to 99% of users while imposing a small burden on the
> contributor (but less of a burden than having to reimplement boost
> features).
>
> I would like the policy to change from “don’t use boost” to “if you use
> boost, make sure compilation of your code is guarded in cmake”.
>
> David Koes
>
> Assistant Professor
> Computational & Systems Biology
> University of Pittsburgh
>
>
> On Jun 11, 2019, at 10:14 AM, Geoffrey Hutchison <
> geoff.hutchi...@gmail.com> wrote:
>
> To start out, I'll emphasize that OB has been a default package on several
> intentionally slow-to-upgrade distros. I'm not surprised to access new
> supercomputing resources to find Open Babel 2.3.2 or something similarly
> old. As a result, I do think backwards compatibility for compilers is
> important.
>
> On Jun 11, 2019, at 5:32 AM, Noel O'Boyle  wrote:
> Well, personally, I don't want that dependency. It would mean dropping
> support for particular legacy OSes, where Open Babel has compiled without
> trouble until now (similarly the CXX11 requrest)
>
>
> Perhaps package maintainers can chime in, but IIRC that C++11 was
> supported back in GCC-4.8.0 from 2013. I don't personally see a problem
> with that going forward.
>
> Turning it around, is it reasonable to add a requirement for Boost just to
> read in a particular file format? I'm happy to work with you to remove
> Boost from your codebase if that would help.
>
>
> I mentioned this to Patrick because we have plenty of formats that are
> turned on or off based on build support (XML, JSON, Eigen, Cairo, etc.)
>
> My personal opinion would be to have a Cmake test for Boost and compile
> the maeparser code conditional on that. At one point we had a boost check
> (for shared_ptr support) and it's definitely how things are organized for
> formats:
>
> if(MSVC OR HAVE_REGEX_H)
> …
>
> endif(MSVC OR HAVE_REGEX_H)
>
> if(WITH_JSON)
>
> (etc).
>
> From Patrick:
>
> my worry there is that package maintainers would default to leaving it off
> and not add boost as a requirement, effectively disabling this feature for
> most users.
>
>
> I don't think we can control what package maintainers do. But if you think
> package maintainers are going to ignore features with Boost, that tells me
> a lot..
>
> For now, I'm fine with Boost as an optional dependency for MAE compilation.
>
> -Geoff
> ___
> OpenBabel-Devel mailing list
> OpenBabel-Devel@lists.sourceforge.net
>
> https://nam05.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.sourceforge.net%2Flists%2Flistinfo%2Fopenbabel-devel&data=02%7C01%7Cdkoes%40pitt.edu%7C68edfea1b7e54f7b0e5408d6ee771c1c%7C9ef9f489e0a04eeb87cc3a526112fd0d%7C1%7C0%7C636958592679903209&sdata=aMfq4jNyCS1oGFTgR4uBbhmMrw8NiJKeUB2f3b8vLe4%3D&reserved=0
>
>
> ___
> OpenBabel-Devel mailing list
> OpenBabel-Devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openbabel-devel
>
___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel


Re: [OpenBabel-Devel] maeparser depends on boost iostreams, can openbabel use it?

2019-06-11 Thread Koes, David
I agree with Geoff for all the reasons listed. Not being able to use boost is a 
constant annoyance.  Making any boost dependent features conditional on boost 
available in cmake seems like a great compromise that will provide the feature 
to 99% of users while imposing a small burden on the contributor (but less of a 
burden than having to reimplement boost features).

I would like the policy to change from “don’t use boost” to “if you use boost, 
make sure compilation of your code is guarded in cmake”.

David Koes

Assistant Professor
Computational & Systems Biology
University of Pittsburgh


On Jun 11, 2019, at 10:14 AM, Geoffrey Hutchison 
mailto:geoff.hutchi...@gmail.com>> wrote:

To start out, I'll emphasize that OB has been a default package on several 
intentionally slow-to-upgrade distros. I'm not surprised to access new 
supercomputing resources to find Open Babel 2.3.2 or something similarly old. 
As a result, I do think backwards compatibility for compilers is important.

On Jun 11, 2019, at 5:32 AM, Noel O'Boyle 
mailto:baoille...@gmail.com>> wrote:
Well, personally, I don't want that dependency. It would mean dropping support 
for particular legacy OSes, where Open Babel has compiled without trouble until 
now (similarly the CXX11 requrest)

Perhaps package maintainers can chime in, but IIRC that C++11 was supported 
back in GCC-4.8.0 from 2013. I don't personally see a problem with that going 
forward.

Turning it around, is it reasonable to add a requirement for Boost just to read 
in a particular file format? I'm happy to work with you to remove Boost from 
your codebase if that would help.

I mentioned this to Patrick because we have plenty of formats that are turned 
on or off based on build support (XML, JSON, Eigen, Cairo, etc.)

My personal opinion would be to have a Cmake test for Boost and compile the 
maeparser code conditional on that. At one point we had a boost check (for 
shared_ptr support) and it's definitely how things are organized for formats:

if(MSVC OR HAVE_REGEX_H)
…
endif(MSVC OR HAVE_REGEX_H)

if(WITH_JSON)
(etc).

From Patrick:
my worry there is that package maintainers would default to leaving it off and 
not add boost as a requirement, effectively disabling this feature for most 
users.

I don't think we can control what package maintainers do. But if you think 
package maintainers are going to ignore features with Boost, that tells me a 
lot..

For now, I'm fine with Boost as an optional dependency for MAE compilation.

-Geoff
___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://nam05.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.sourceforge.net%2Flists%2Flistinfo%2Fopenbabel-devel&data=02%7C01%7Cdkoes%40pitt.edu%7C68edfea1b7e54f7b0e5408d6ee771c1c%7C9ef9f489e0a04eeb87cc3a526112fd0d%7C1%7C0%7C636958592679903209&sdata=aMfq4jNyCS1oGFTgR4uBbhmMrw8NiJKeUB2f3b8vLe4%3D&reserved=0

___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel


Re: [OpenBabel-Devel] maeparser depends on boost iostreams, can openbabel use it?

2019-06-11 Thread Koes, David
I would be in favor of making boost a default that has to be explicitly 
disabled with a cmake define (but still require building w/o boost to work).  
The failure message would tell the user what they have to pass to disable the 
boost requirement.

David Koes

Assistant Professor
Computational & Systems Biology
University of Pittsburgh


On Jun 11, 2019, at 11:18 AM, Patrick Lorton 
mailto:klor...@gmail.com>> wrote:

OK Geoff - sounds good, I'll change the code so that it checks for boost and 
only turns maeparser on if boost is present.

To clarify my worries about package maintainers, it's not that they'd have a 
problem with turning boost on, it's just that if we don't make the default 
build hard fail w/o boost, they won't even investigate and think to add boost 
as a requirement.  Maybe I'm not giving them enough credit :)  Or maybe I 
should just pester the major package maintainers to make sure they realize 
after this goes in.

There is one issue that remains around this that I can think of:  The GitHub 
builders.  Do we want to add boost there so they test building maestro and 
running the tests?  Or do you run multiple builders with different settings?  
I'm happy to help there, but if there's someone who owns this for you, I'm also 
happy to just raise the flag and move on.

Pat

___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel


Re: [OpenBabel-Devel] maeparser depends on boost iostreams, can openbabel use it?

2019-06-11 Thread Patrick Lorton
OK Geoff - sounds good, I'll change the code so that it checks for boost
and only turns maeparser on if boost is present.

To clarify my worries about package maintainers, it's not that they'd have
a problem with turning boost on, it's just that if we don't make the
*default* build hard fail w/o boost, they won't even investigate and think
to add boost as a requirement.  Maybe I'm not giving them enough credit :)
Or maybe I should just pester the major package maintainers to make sure
they realize after this goes in.

There is one issue that remains around this that I can think of:  The
GitHub builders.  Do we want to add boost there so they test building
maestro and running the tests?  Or do you run multiple builders with
different settings?  I'm happy to help there, but if there's someone who
owns this for you, I'm also happy to just raise the flag and move on.

Pat

PS - one additional heads up:  this summer of code project

will
add dependencies to maeparser/Boost, and I think some Qt dependencies.

On Tue, Jun 11, 2019 at 10:14 AM Geoffrey Hutchison <
geoff.hutchi...@gmail.com> wrote:

> To start out, I'll emphasize that OB has been a default package on several
> intentionally slow-to-upgrade distros. I'm not surprised to access new
> supercomputing resources to find Open Babel 2.3.2 or something similarly
> old. As a result, I do think backwards compatibility for compilers is
> important.
>
> On Jun 11, 2019, at 5:32 AM, Noel O'Boyle  wrote:
> Well, personally, I don't want that dependency. It would mean dropping
> support for particular legacy OSes, where Open Babel has compiled without
> trouble until now (similarly the CXX11 requrest)
>
>
> Perhaps package maintainers can chime in, but IIRC that C++11 was
> supported back in GCC-4.8.0 from 2013. I don't personally see a problem
> with that going forward.
>
> Turning it around, is it reasonable to add a requirement for Boost just to
> read in a particular file format? I'm happy to work with you to remove
> Boost from your codebase if that would help.
>
>
> I mentioned this to Patrick because we have plenty of formats that are
> turned on or off based on build support (XML, JSON, Eigen, Cairo, etc.)
>
> My personal opinion would be to have a Cmake test for Boost and compile
> the maeparser code conditional on that. At one point we had a boost check
> (for shared_ptr support) and it's definitely how things are organized for
> formats:
>
> if(MSVC OR HAVE_REGEX_H)
> …
>
> endif(MSVC OR HAVE_REGEX_H)
>
> if(WITH_JSON)
>
> (etc).
>
> From Patrick:
>
> my worry there is that package maintainers would default to leaving it off
> and not add boost as a requirement, effectively disabling this feature for
> most users.
>
>
> I don't think we can control what package maintainers do. But if you think
> package maintainers are going to ignore features with Boost, that tells me
> a lot..
>
> For now, I'm fine with Boost as an optional dependency for MAE compilation.
>
> -Geoff
>
___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel


Re: [OpenBabel-Devel] maeparser depends on boost iostreams, can openbabel use it?

2019-06-11 Thread Geoffrey Hutchison
To start out, I'll emphasize that OB has been a default package on several 
intentionally slow-to-upgrade distros. I'm not surprised to access new 
supercomputing resources to find Open Babel 2.3.2 or something similarly old. 
As a result, I do think backwards compatibility for compilers is important.

> On Jun 11, 2019, at 5:32 AM, Noel O'Boyle  wrote:
> Well, personally, I don't want that dependency. It would mean dropping 
> support for particular legacy OSes, where Open Babel has compiled without 
> trouble until now (similarly the CXX11 requrest)

Perhaps package maintainers can chime in, but IIRC that C++11 was supported 
back in GCC-4.8.0 from 2013. I don't personally see a problem with that going 
forward.

> Turning it around, is it reasonable to add a requirement for Boost just to 
> read in a particular file format? I'm happy to work with you to remove Boost 
> from your codebase if that would help.

I mentioned this to Patrick because we have plenty of formats that are turned 
on or off based on build support (XML, JSON, Eigen, Cairo, etc.)

My personal opinion would be to have a Cmake test for Boost and compile the 
maeparser code conditional on that. At one point we had a boost check (for 
shared_ptr support) and it's definitely how things are organized for formats:

> if(MSVC OR HAVE_REGEX_H)
> …
> endif(MSVC OR HAVE_REGEX_H)
> 
> if(WITH_JSON)
(etc).

From Patrick:
> my worry there is that package maintainers would default to leaving it off 
> and not add boost as a requirement, effectively disabling this feature for 
> most users.


I don't think we can control what package maintainers do. But if you think 
package maintainers are going to ignore features with Boost, that tells me a 
lot..

For now, I'm fine with Boost as an optional dependency for MAE compilation.

-Geoff___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel


Re: [OpenBabel-Devel] maeparser depends on boost iostreams, can openbabel use it?

2019-06-11 Thread Patrick Lorton
Hi Noel - after discussing it more, I think we're pretty committed to
having a Boost dependency in maeparser.  Even with our current boost
requirement, maeparser already compiles on where we "draw the line" for old
platforms:  CentOS 6, released 8 years ago, the oldest CentOS still getting
security updates, 1 year left until EOL.  We like the functionality Boost
provides and don't really want to reinvent all the wheels we'd have to in
order to rip it out, as it does seem pretty standard at this point.

My proposal is not that it'd be difficult to build or package OpenBabel
without Boost/maeparser, just that if you wanted to do so you'd have to
explicitly add -DMAEPARSER=off to the cmake build.  For really old
platforms or those that wanted to build without the boost dependency, I
think that's still pretty straightforward.  This would also disable the
C++11 requirement as well.

The other option is to have it default to -DMAEPARSER=off, but from our
experience I do honestly believe far far more users have boost or have
boost trivially available than don't, so I'm (for obvious reasons) biased
towards having it exposed by default.  Splitting the baby would be to check
for Boost and turn off maeparser if Boost isn't available, my worry there
is that package maintainers would default to leaving it off and not add
boost as a requirement, effectively disabling this feature for most users.

Pat



On Tue, Jun 11, 2019 at 5:32 AM Noel O'Boyle  wrote:

> Well, personally, I don't want that dependency. It would mean dropping
> support for particular legacy OSes, where Open Babel has compiled without
> trouble until now (similarly the CXX11 requrest). As I see it, users of
> Open Babel will see no benefit from us using Boost, but will find it more
> difficult to compile.
>
> Turning it around, is it reasonable to add a requirement for Boost just to
> read in a particular file format? I'm happy to work with you to remove
> Boost from your codebase if that would help.
>
> Regards,
> - Noel
>
> On Mon, 10 Jun 2019 at 13:45, Patrick Lorton  wrote:
>
>> Hello all - I just put together a pull request
>> to add maestro file
>> parsing support (using maeparser
>> ) to OpenBabel.  Geoff pointed
>> out in the pull request that Boost is not currently a requirement of
>> OpenBabel, and that it was not clear if it's a good idea to make it one.
>>
>> I'm curious what the current opinions would be towards making Boost a
>> requirement of OpenBabel?  RDKit has had a semi-optional Boost requirement
>> for a while (including for its use of maeparser now), so OpenBabel wouldn't
>> be alone amongst Chemistry packages.
>>
>> As long as someone's not using cutting edge Boost features, it seems a
>> guarantee for package managers to have working versions of Boost available
>> (including Conda of course).
>>
>> There's some 'in the middle' options: detecting whether Boost is already
>> installed, and only building Maestro support in if it's already available;
>> or having maeparser just off by default and allow people to turn it on
>> (also turning Boost requirement on).  I think those options would
>> effectively turn maeparser off for the vast majority of users who would be
>> using OpenBabel from a package built with minimum dependencies.
>>
>> I'm personally advocating for adding a default-boost-requirement that
>> could be turned off by turning maeparser off, but am of course at the
>> service of the project owners :)
>>
>> Looking forward to any thoughts/discussions,
>> Pat
>> ___
>> OpenBabel-Devel mailing list
>> OpenBabel-Devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/openbabel-devel
>>
>
___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel


Re: [OpenBabel-Devel] maeparser depends on boost iostreams, can openbabel use it?

2019-06-11 Thread Noel O'Boyle
Well, personally, I don't want that dependency. It would mean dropping
support for particular legacy OSes, where Open Babel has compiled without
trouble until now (similarly the CXX11 requrest). As I see it, users of
Open Babel will see no benefit from us using Boost, but will find it more
difficult to compile.

Turning it around, is it reasonable to add a requirement for Boost just to
read in a particular file format? I'm happy to work with you to remove
Boost from your codebase if that would help.

Regards,
- Noel

On Mon, 10 Jun 2019 at 13:45, Patrick Lorton  wrote:

> Hello all - I just put together a pull request
> to add maestro file
> parsing support (using maeparser
> ) to OpenBabel.  Geoff pointed
> out in the pull request that Boost is not currently a requirement of
> OpenBabel, and that it was not clear if it's a good idea to make it one.
>
> I'm curious what the current opinions would be towards making Boost a
> requirement of OpenBabel?  RDKit has had a semi-optional Boost requirement
> for a while (including for its use of maeparser now), so OpenBabel wouldn't
> be alone amongst Chemistry packages.
>
> As long as someone's not using cutting edge Boost features, it seems a
> guarantee for package managers to have working versions of Boost available
> (including Conda of course).
>
> There's some 'in the middle' options: detecting whether Boost is already
> installed, and only building Maestro support in if it's already available;
> or having maeparser just off by default and allow people to turn it on
> (also turning Boost requirement on).  I think those options would
> effectively turn maeparser off for the vast majority of users who would be
> using OpenBabel from a package built with minimum dependencies.
>
> I'm personally advocating for adding a default-boost-requirement that
> could be turned off by turning maeparser off, but am of course at the
> service of the project owners :)
>
> Looking forward to any thoughts/discussions,
> Pat
> ___
> OpenBabel-Devel mailing list
> OpenBabel-Devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openbabel-devel
>
___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel