working image for pymc-marketing pre installed env #341
Conversation
|
now users can spin up the container using : |
|
I think you only need two lines in the Dockerfile: FROM jupyter/minimal-notebook:latest
RUN mamba install pymc-marketingAnd you don't need |
|
oh, actually it does work with the 2 liner, now thinking about it I don't know why I was creating virtualenv for installation within the container :) |
Codecov Report
@@ Coverage Diff @@
## main #341 +/- ##
=======================================
Coverage 95.85% 95.85%
=======================================
Files 19 19
Lines 1447 1447
=======================================
Hits 1387 1387
Misses 60 60 |
|
One thing to note here is that this won't give you any sort of reproducibility. You might want to consider building the Docker image in CI. This has the bonus that downloading is much faster than building. You might also consider some CI to ensure that the notebooks all execute without error. (As dependencies update, things sometimes break.) Then at least you'd know when things stop working. |
|
Do you mean like creating a CI that would build and push the image to the dockerhub and then get the users to pull and spin that up? |
|
Ya. I'd probably recommend GitHub Container Registry over DockerHub though. |
|
Yes, that looks about right, at least for the tricky half of it. Great work! The other half which I'd suggest implementing is to push to GHCR (GitHub Container Registry) after the "Check for errors" step succeeds. I'd recommend adding an Finally, regarding the scheduled tests, I believe that when they fail they will do so silently, so you will probably want to look into some notification mechanism, e.g. open an issue when the workflow fails. |
|
Ah, I'd also add a "workflow dispatch" (i.e. a button) trigger so that you can kick things off manually. |
|
@ricardoV94 , @twiecki so far I'm including only the latest deployment notebook in the CI (I assume that the env requirements should be the same for all of the notebooks in the repo) but if my assumption is incorrect please list the notebooks we should be checking in here (I think we should avoid checking every single notebook in one job as it would make the runtime very long) |
|
I don't know offhand how to open an issue on failure, but this looks promising. You'll need an if clause which looks something like: if: failure() && (github.event_name == 'schedule') |
|
Can we confirm this has been tested for compilation errors? https://discourse.pymc.io/t/pytensor-compilation-error/11047 If the above message appears when I've encountered this with every dev container variation I've tried in GitHub codespaces (which are Debian builds). However, I noticed the latest Ubuntu image is being pulled in the Dockerfile, so it's worth verifying. Using These compilation errors are one of the most common issues encountered in
|
|
I just want to point out that this PR is an excellent initiative 💪 ! |
|
Woah @BBDS-Colt, that's some huge type! I totally agree, and no worries, I've got you covered. Check out the code I wrote a few years ago (before the whole Aesara/PyTensor fork) here. |
|
Ah, on second thought it's not quite that easy for Apple Silicon... GitHub doesn't provide ARM runners, so if you want to run tests you'd have to emulate with QEMU. It's doable but a little more complicated. |
|
@BBDS-Colt Locally I had the same issue, I found that when setting up the conda env on Mac M1 you should install all the dependencies, then uninstall the pytensor-base and install it again using conda or mamba install. The reason I suspect is that installing ipykernel re-installs some dependency package that doesn't support the arm architecture, therefore making pytensor unusable on arm. Reinstallation helped in my case, but working with the Jupyter-minimal image didn't cause the same issues to arise. I think in that case, all the packages were installed in the base image, and then we're installing pymc-marketing with correct arm-supporting dependencies |
@michaelraczycki, this seems like a bug. Would you be able to file a report, perhaps at PyTensor's conda-forge feedstock? |
|
Didn't you also have some crazy recipe on Discourse for the BLAS setup? Could you also create an issue for that? I want to build it into the feedstock when I get the chance. |
Actually I don't remember the BLAS case, I can try to find it but I think it was also connected to the same pytensor issue. I can open the issue at Conda-forge pytensor, but first I'd try to reproduce the error again, it's been few months since I've last time seen it |
was this meant as a follow up to your conversation with Colt, or to our one about the Git CI for checking the notebooks? |
|
More directly it was a followup with Colt, but we'd probably implement it in the CI. There's quite a lot to do here, so perhaps it makes sense to merge this as a start and break up the other suggestions into a bunch of enhancements. |
I agree; let's keep this one focused and leave the BLAS contingencies for a future PR. If you want me to post any issues for what's been discussed, let me know. Also, this library runs flawlessly with |
|
Ya, the BLAS issues are anyways specific to PyTensor and don't have anything directly to do with this. |
|
Is there a consensus whether this is still useful or can we close? |
|
Looks useful, I say let's merge it. |
|
I don't want a new CI to test 2 lines of code if that's okay |
That's okay ;). |
|
There's a lot happening in this PR, so probably best to regroup under more targeted efforts. Potentially:
|
This PR includes a Dockerfile to the repo, which allows users to build an image with working jupyter lab environment, pre-installed with all necessary libraries in order to run the tutorial notebooks. Because of that no local environment creation is needed.
📚 Documentation preview 📚: https://pymc-marketing--341.org.readthedocs.build/en/341/