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.

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

Allow install from conda repository directly #144

Closed
jfogel-dwave opened this issue Sep 2, 2021 · 30 comments
Closed

Allow install from conda repository directly #144

jfogel-dwave opened this issue Sep 2, 2021 · 30 comments
Labels

Comments

@jfogel-dwave
Copy link

Current Problem
In order to install dwave-ocean-sdk in a conda environment it is necessary to first install pip (and related packages) and then use that to install dwave-ocean-sdk.

Proposed Solution
Get dwave-ocean-sdk added to the conda or conda-forge repository so that a user can do a simple conda install dwave-ocean-sdk in order to install.

@mhlr
Copy link

mhlr commented Sep 12, 2021

It would be great have D-Wave available in conda. Being i conda should ivrease the visibility and adoption of D-Wave tools among scientific users of Python who often promarily use conda. Some advantages of conda are

  • conda provides MKL compiled numerical libraries by default, which in turn means that native vectorised numpy/scipy operation run multithreaded by default (without GIL), so the same code can run much faster in a conda based environment than in a pip based one if depends on BLAS based libraries.
  • a number of math/science/ai/ml packages are only in conda and not pypi.
  • the official repos are curated so the quality tends to be higher than pypi.
  • the conda (& now also mamba) package manager maintains global environment consistency so one install does not break previously installed packages. Conda will (up/down) grade previouly installed packages if necessary.
  • conda is also language agnostic so you can distribute C, R, Julia, Rust packages with it. Main drawback is it can not install directly from git.

@BastianZim
Copy link

Hi everybody, I've coincidentally started working on the sdk separate from this request as I needed it myself but happy to join forces.
The big problem currently is the or-tools dependency as that's a bit of a beast (conda-forge/staged-recipes#16147) but once that's in conda-forge we should be able to bring the sdk to conda-forge as well.

@BastianZim
Copy link

BastianZim commented Oct 31, 2021

PR is at conda-forge/staged-recipes#16694 but will take some time.
I will add some questions here in the meanwhile when I stumble upon them, would be great if someone from here could weigh in. Thanks!

@BastianZim
Copy link

Some packages use the following:

cwd = os.path.abspath(os.path.dirname(__file__))
if not os.path.exists(os.path.join(cwd, 'PKG-INFO')):
    try:
        from Cython.Build import cythonize
        USE_CYTHON = True
    except ImportError:
        USE_CYTHON = False
else:
    USE_CYTHON = False

Could someone explain what is happening here? Should I use Cython or not?

@BastianZim
Copy link

Would someone from here like to co-maintain them?

@BastianZim
Copy link

BastianZim commented Oct 31, 2021

Some packages have files that are licensed differently than the overall package. Does anyone have some more info here on whether that is to stay and if that should be explicitly mentioned?

  • dwave-tabu at src/extern/stdint.h
  • dwave-qbsolv at src/wingetopt.h

Also, are there any other files like this?

@BastianZim
Copy link

BastianZim commented Oct 31, 2021

minorminer contains rectangle-packer which is quite a bit against conda-forge policies as it's preferred to ship packages individually. I'd therefore separate them and submit both. Or is there a reason for this?

@BastianZim
Copy link

pyqubo contains cpp_dimod, googletest and pybind11. googletest is only used for testing as far as I can see but is the rest compiled and used? In that case we would have to separate them and use the conda-forge versions.

@BastianZim
Copy link

ortools is currently fixed to >=6.6.4659,<9.0.0 but conda-forge only has 9.1 due to difficulties with the build system. Can that also be used?

@randomir
Copy link
Member

randomir commented Nov 1, 2021

Hi @BastianZim,

Thanks for doing this!

I'll answer your questions, in order, here.


Some packages use the following:

cwd = os.path.abspath(os.path.dirname(__file__))
if not os.path.exists(os.path.join(cwd, 'PKG-INFO')):
    try:
        from Cython.Build import cythonize
        USE_CYTHON = True
    except ImportError:
        USE_CYTHON = False
else:
    USE_CYTHON = False

Could someone explain what is happening here? Should I use Cython or not?

This is a deprecated workaround we used many years ago. Some packages have we modernized to use PEP-517 (see dwave-greedy, dwave-tabu, dimod), but some still haven't. They will in time.

Short answer: use Cython.


Would someone from here like to co-maintain them?

That would be great. Someone from the Tools team. I can volunteer myself (@randomir), but we can wait for @arcondello and @hhtong to chime in. Alternatively, we're happy (and grateful!) with you maintaining it, if you have the bandwidth for that. We can ping you before (or shortly after) we release a new version of the SDK.


Some packages have files that are licensed differently than the overall package. Does anyone have some more info here on whether that is to stay and if that should be explicitly mentioned?

* `dwave-tabu` at _src/extern/stdint.h_

* `dwave-qbsolv` at _src/wingetopt.h_

Also, are there any other files like this?

That's a good catch. These are to support Windows builds.
We might easily get rid of the stdint.h in dwave-tabu. @hhtong, would you be able to verify we still need this and remove it if possible?
I believe we can remove wingetopt.h as well, but again, need to verify. I can do that.


minorminer contains rectangle-packer which is quite a bit against conda-forge policies as it's preferred to ship packages individually. I'd therefore separate them and submit both. Or is there a reason for this?

@boothby, can you answer this one, please?


pyqubo contains cpp_dimod, googletest and pybind11. googletest is only used for testing as far as I can see but is the rest compiled and used? In that case we would have to separate them and use the conda-forge versions.

@kotarotanahashi is pyqubo author/maintainer, so I'll let him answer this.


ortools is currently fixed to >=6.6.4659,<9.0.0 but conda-forge only has 9.1 due to difficulties with the build system. Can that also be used?

I'm not sure, we'll have to check. IIRC, 9.0+ does not work for us, or at least not until we update the pysmt version. I can check that.

@boothby
Copy link

boothby commented Nov 1, 2021

I direct-included rectangle-packer because they were not shipping (macOS?) wheels at the time that I wanted to release with that as a dependency. It's quite a ways down my todo list, but it shouldn't be much work to revert that inclusion.

@BastianZim
Copy link

Hi @randomir (Great profile pic by the way 😄),

I will answer in a list just to make it easier.

  1. Ok. Will require a bit of trial and error here because I'm not sure how much is actually required and how much is leftover but will figure that out. I generated most packages with Grayskull which is quite good at figuring out hidden dependencies but it might've been overeager in this case.
  2. Happy to maintain it and also have the bandwidth. The reason I asked is that sometimes companies like to at least co-maintain their packages but this is not a requirement. I can add maintainers at any time so just let me know at some point if you'd like to co-maintain and who should that be (Can also be multiple people) and I'll add everyone.
  3. Ok will wait for your feedback RE licenses.
  4. Thanks for the clarification @boothby. For now, I think I can just rm the folder and substitute the conda-forge package. There are some other things I need to fix first before I can test this but it should work, I would assume.
  5. Ok will wait, thanks.
  6. Ok. 9.0+ support would be GREAT because building ortools is quite difficult (One of the most difficult ones I've encountered so far actually 😅) and most of the painful points were fixed with the 9.0 release so it would be great if we could keep that.

@boothby
Copy link

boothby commented Nov 1, 2021

You can't just delete the folder; it's a C extension and the build will fail if it doesn't exist. I'll take care of this today.

@BastianZim
Copy link

Oh yeah, you're right - didn't check all folders. Thanks!

@BastianZim
Copy link

One other problem: dimod depends on dwave-preprocessing which depends on dimod which makes it really difficult to add since conda builds them sequentially and needs the dependency to be available.

Is there a way to use them without the dependency on each other? Otherwise, I'll have to see if there's a workaround.

@randomir
Copy link
Member

randomir commented Nov 1, 2021

Sounds good, @BastianZim. Thank you!

FWIW, we can fix/resolve most of these for the next minor release of Ocean, if you're willing to wait a few weeks.

The dimod/dwave-preprocessing circular dependency is a known issue. We should prioritize resolving that. I think you already started working on it, @arcondello?

One (short-term) workaround is to use an older dimod for building dwave-preprocessing, see dwavesystems/dwave-preprocessing#28.

@BastianZim
Copy link

Of course, no timeline on my side I'm just tinkering around. Happy to wait until you're ready. Would you mind pinging here once everything is published?

Ok, would completely follow your lead here about which approach to choose RE dimod.

@BastianZim
Copy link

Hi everybody, just wanted to check in and see if there have been any developments that would allow us to start the packaging process or if we should still wait further?

@arcondello
Copy link
Member

Hi @BastianZim , thanks for the great work! If you want to maintain, that would be great!

@randomir
Copy link
Member

Hey @BastianZim, thanks again for doing this!

To answer your question:

So, you're not fully unblocked yet, but some progress is possible.

@BastianZim
Copy link

Hi @randomir thanks for your reply!

  1. Yes, thank you!
  2. Thanks
  3. OK then I'll keep everything depending on that out for now
  4. Great, thanks. Are you aware of any other packages where dependencies are directly distributed with the project?
  5. Since the upper boundary was increased to 10 there is no problem from the conda-forge side anymore. We have ortools available on Linux now (and hopefully soon Mac) so no impact on our side.

@randomir
Copy link
Member

Hi @BastianZim,

Re: 4, no, IIRC. The closest to that is a custom setup step during which we install one of the pysmt solvers (z3) during penaltymodel-maxgap install. But we are dropping this in dwavesystems/penaltymodel#138 as well. A tentative release date is now known - early April. /cc @arcondello.

Re: 5, Ocean 5, early April.

@BastianZim
Copy link

Hi everybody, I'm making good progress but am stuck on conda-forge/staged-recipes#18266. Could someone please have a look at it?
Thanks!

@BastianZim
Copy link

BastianZim commented Apr 5, 2022

Hi everybody,

I've now made some more progress and have added all of the packages that are not blocked by something.

The following are still missing:

@BastianZim
Copy link

Hi everyone, since the new release doesn't depend on Pyqubo anymore, I have submitted this package and it was just merged.
Our CIs take a couple of hours but the package can now be installed through conda-forge so the issue can be closed.

@randomir
Copy link
Member

Great news! Thanks for contributing Ocean to Conda, @BastianZim!

Just note that the pyqubo removal is temporary, until they update it for dimod 0.11.

@BastianZim
Copy link

My pleasure!

Yeah and I'm tackling that now but since it's not included I wanted to get this out before. Do you have any connections to the author of it?

@randomir
Copy link
Member

Here's our issue on pyqubo, for tracking: recruit-communications/pyqubo#150.

@BastianZim
Copy link

Ok had a look and it seems like pyqubo is very straightforward so I'll add that now so that we're prepared for the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants