[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[IMP-dev] Re: build error related to C++11 flag on Mac + conda



Ok, so I will run all tests and push the switch from boost::mt19337 to
std::mt19337 later this week, since this seems to make sense regardless. I
think it should not affect or even improve speed, I will test it on a unix
system tomorrow.

P. S. for for some reason, conda-forge chose version 1.74 from a completely
fresh build. Not sue why - I will see what's up with that...

Thanks!
Barak

On Mon, Dec 19, 2022, 9:55 PM Ben Webb <> wrote:

> On 12/19/22 11:35 AM, Barak Raveh wrote:
> > At least on my Mac installation, conda's gcc package seems to be
> > broken.
>
> Are you sure you have the latest versions of everything? IMP built just
> fine on Mac with conda-forge just 4 days ago, e.g.
>
> https://urldefense.com/v3/__https://dev.azure.com/conda-forge/84710dde-1620-425b-80d0-4cf5baca359d/_apis/build/builds/629436/logs/83__;!!LQC6Cpwp!toOhwAjuC2HImq1BVO0cMwaT7btLidq_BUzSFKiyIhvgeLSTq5Mib1QaLUogxcxqVMFUWCKWtttMKhvj63Q7CQ$ 
>
> > a conflict between conda's boost::mt19337::min()/max() functions and
> > llvm's std::shuffle(), which requires them to return a constexpr (this
> > was fixed in the latest boost versions)
>
> Yes, we had the same issue with Ubuntu, which has an older Boost:
> https://urldefense.com/v3/__https://github.com/salilab/imp/commit/f1b873faeee__;!!LQC6Cpwp!toOhwAjuC2HImq1BVO0cMwaT7btLidq_BUzSFKiyIhvgeLSTq5Mib1QaLUogxcxqVMFUWCKWtttMKhsjzF_SmA$ 
>
> But this issue was fixed in Boost 1.75 and if you're using conda-forge
> you should have 1.78 as the IMP conda-forge package is pinned:
>
> https://urldefense.com/v3/__https://github.com/conda-forge/conda-forge-pinning-feedstock/blob/main/recipe/conda_build_config.yaml__;!!LQC6Cpwp!toOhwAjuC2HImq1BVO0cMwaT7btLidq_BUzSFKiyIhvgeLSTq5Mib1QaLUogxcxqVMFUWCKWtttMKht93U1Enw$ 
>
> > I tested for speed, and std::mt19337 is as fast or even faster on my
> > mac. This is the change, l did not dare to check it in. Let me know if
> > you think we should stick with the boost version or try to add this
> > change
>
> Looks like your proposed change only affects clang builds. If we're
> going to switch to std::mt19337 we should just do so on all platforms,
> since IMP requires C++11 anyway. If there are some weird platforms where
> it doesn't work, we can always #ifdef it there and fall back to Boost.
>
>         Ben
> --
>                       https://salilab.org/~ben/
> "It is a capital mistake to theorize before one has data."
>         - Sir Arthur Conan Doyle
>


Ok, so I will run all tests and push the switch from boost: : mt19337 to std: : mt19337 later this week, since this seems to make sense regardless. I think it should not affect or even improve speed, I will test it on a unix system tomorrow.  P. 
ZjQcmQRYFpfptBannerStart
This Message Is From an External Sender
This message came from outside your organization.
 
ZjQcmQRYFpfptBannerEnd
Ok, so I will run all tests and push the switch from boost::mt19337 to std::mt19337 later this week, since this seems to make sense regardless. I think it should not affect or even improve speed, I will test it on a unix system tomorrow. 

P. S. for for some reason, conda-forge chose version 1.74 from a completely fresh build. Not sue why - I will see what's up with that...

Thanks!
Barak

On Mon, Dec 19, 2022, 9:55 PM Ben Webb <">> wrote:
On 12/19/22 11:35 AM, Barak Raveh wrote:
> At least on my Mac installation, conda's gcc package seems to be
> broken.

Are you sure you have the latest versions of everything? IMP built just
fine on Mac with conda-forge just 4 days ago, e.g.
https://dev.azure.com/conda-forge/84710dde-1620-425b-80d0-4cf5baca359d/_apis/build/builds/629436/logs/83

> a conflict between conda's boost::mt19337::min()/max() functions and
> llvm's std::shuffle(), which requires them to return a constexpr (this
> was fixed in the latest boost versions)

Yes, we had the same issue with Ubuntu, which has an older Boost:
https://github.com/salilab/imp/commit/f1b873faeee

But this issue was fixed in Boost 1.75 and if you're using conda-forge
you should have 1.78 as the IMP conda-forge package is pinned:
https://github.com/conda-forge/conda-forge-pinning-feedstock/blob/main/recipe/conda_build_config.yaml

> I tested for speed, and std::mt19337 is as fast or even faster on my
> mac. This is the change, l did not dare to check it in. Let me know if
> you think we should stick with the boost version or try to add this
> change

Looks like your proposed change only affects clang builds. If we're
going to switch to std::mt19337 we should just do so on all platforms,
since IMP requires C++11 anyway. If there are some weird platforms where
it doesn't work, we can always #ifdef it there and fall back to Boost.

        Ben
--
" target="_blank" rel="noreferrer">                      https://salilab.org/~ben/
"It is a capital mistake to theorize before one has data."
        - Sir Arthur Conan Doyle