Skip to content

Add an option to make frontend for not unintalling current environment's PL #2914

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

Open
paul0403 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add an option to make frontend for not unintalling current environment's PL #2914

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9cbf9ec
Add an option to `make frontend` for not unintalling current environm…
paul0403 Jun 4, 2026
7fc5f5b
Merge remote-tracking branch 'origin/main' into make_frontend_opt_nou…
paul0403 Jun 4, 2026
33c0c38
no deps
paul0403 Jun 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ RT_BUILD_DIR ?= $(MK_DIR)/runtime/build
OQC_BUILD_DIR ?= $(MK_DIR)/frontend/catalyst/third_party/oqc/src/build
ENZYME_BUILD_DIR ?= $(MK_DIR)/mlir/Enzyme/build
COVERAGE_REPORT ?= term-missing
UNINSTALL_PL ?= ON

@dime10 dime10 Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this! Slightly clearer naming might be the following, because uninstall sounds like we might plain remove it?

Suggested change
UNINSTALL_PL ?= ON
FORCE_PL_INSTALL ?= ON

@maliasadi maliasadi Jun 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a description and update the help command?

ENABLE_OPENQASM ?= ON
ENABLE_OQD ?= OFF
TEST_BACKEND ?= "lightning.qubit"
Expand Down Expand Up @@ -118,10 +119,14 @@ catalyst: runtime dialects plugin frontend oqc
.PHONY: frontend
frontend:
@echo "install Catalyst Frontend"
ifeq ($(UNINSTALL_PL),ON)
# Uninstall pennylane before updating Catalyst, since pip will not replace two development
# versions of a package with the same version tag (e.g. 0.38-dev0).
$(PYTHON) -m pip uninstall -y pennylane
$(PYTHON) -m pip install -e . --extra-index-url https://test.pypi.org/simple $(PIP_VERBOSE_FLAG)
else
$(PYTHON) -m pip install -e . --no-deps

@paul0403 paul0403 Jun 4, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mudit2812 I tried locally, setup.py will still overwrite specific PL branch.

Say the current .dep-versions uses PL version dev10. So we have a couple cases:

  • The developer's PL branch is less than dev10. In these cases, setup.py will still uninstall and bump it up to dev10 on testpypi.
  • The developer's PL is >= dev10, in which case it is kept, and setup.py does nothing.

If we use --no-deps like I did here, we guarantee that the uninstall absolutely does not happen, but this does assume make frontend is only run after pip install -r requirements.txt. I do think this is a fair assumption, since this is the recommendation in the build guide.

However, I see the other side too: if a PL branch is too old, it should probably be forwarded by the dev.

What do you think?

@dime10 dime10 Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah now that I think about it, the uninstall step merely forces picking up the exact PL version required by Catalyst when the installed package already has the same version tag (sometimes this happens, same tag, but different code). Disabling this does not affect the case where the installed PL version has a different tag, in which case pip will still look for the one provided by catalyst.

The only way around that that I know is --no-deps, we can do that but you can't assume just because requirement.txt was installed that we don't need the deps declared by the Catalyst package. In our setup, requirements.txt are build-time requirements, which are not the same as runtime requirements. Not installing the latter could lead to wonky configurations (e.g. a wrong lightning install, missing xdsl or malt package, etc.).

Of course if you know what you're doing you can proceed at your own risk, but you don't need a separate flag for that tbh, just run pip install -e . --no-deps.

@mudit2812 mudit2812 Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a discussion with @dime10 , and the two most favourable solutions seemed to be the following:

  • Just add a note to the build from source guide saying that make frontend deletes the local installation of PennyLane and it must be reinstalled if needed.
  • Update the implementation of the new flag to look for a preinstalled version of pennylane (using pip list) and then restoring it after installing Catalyst. This one is a bit more complicated, but may provide a better UX

endif
$(PYTHON) -m catalyst.utils.precompile_decomposition_rules
rm -r frontend/pennylane_catalyst.egg-info

Expand Down
Loading