Skip to content

Conversation

Copy link

Copilot AI commented Oct 2, 2025

  • Understand the current compiler detection logic in pytensor/configdefaults.py
  • Modify add_compile_configvars() to check CXX environment variable first before falling back to which search
  • Add test to verify CXX environment variable is respected
  • Verify existing tests still pass
  • Test the fix manually with CXX environment variable set
  • Update documentation to mention CXX environment variable support
  • Add comprehensive tests verifying correct priority order (explicit config > CXX > which-search)
Original prompt

Please implement "the right thing" as per the below discussion.


Ben Mares

I'm working on PyTensor where we use conda-forge to set up users with a C toolchain for runtime compilation. I currently include the compiler('c') macro in the run requirements, but that's been causing failures lately. In particular, when users upgrade to macOS 15.4, they get

ld: -lto_library library filename must be 'libLTO.dylib'

This can be mitigated by installing c-compiler or its missing dependency ld64. I know we are not supposed to use c-compiler or cxx-compiler in recipes, but I'm wondering if that advice applies only to build/host sections?

I have a PR ready to merge that includes ld64 as a runtime dependency that seems to fix the issue, but I'm concerned that this fix may not be principled. Is this okay, or should I be doing it differently?

References:
PR for adding ld64 on osx as a run dep: conda-forge/pytensor-suite-feedstock#176
PyTensor issue of the original ld error: pymc-devs#1342
Previous (dead) Zulip thread on the topic: https://conda-forge.zulipchat.com/#narrow/channel/457337-general/topic/Unsure.20if.20bug.3A.20can't.20use.20LTO.20with.20Apple.20linker.20.28clangdev.29


Isuru Fernando

Does PyTensor ignore CXX env variable?


Cove G

@isuru Fernando Interesting question. I think the answer is yes, pytensor does ignore the CXX variable. Pytensor seems to instead find the cxx binary (being clang++ on macOS) by doing a which-like search. This uses the clang++ binary provided by the conda environment, but doesn't end up referencing the CXX variable at all. (supporting lines in pytensor source)

Would that be an issue? It's still the case that ld64 isn't included in the conda environment despite asking for compiler('cxx').


Chris Burr

by doing a which-like search

I would probably patch that logic to make it do the right thing for conda-forge


Isuru Fernando: Working on this at conda-forge/clangdev-feedstock#390, but it'd be good to use the CXX env variable in pytensor


Ben Mares: Thanks @isuru Fernando! I also help to maintain PyTensor (which used to be Theano), so we have lots of legacy stuff. I'm happy to support making sure we do "the right thing". Is that just a matter of preempting the which search with CXX when it exists? Or is it more subtle?


Isuru Fernando: Yes, that's it


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title Please implement "the right thing" as per the below discussion. --- Ben Mares I'm working on PyTensor where we use conda-forge to set up users with a C toolchain for runtime compilation. I currently include the compiler('c') macro in the run requi... Respect CXX environment variable for C++ compiler selection Oct 2, 2025
Copilot AI requested a review from maresb October 2, 2025 17:57
Copilot finished work on behalf of maresb October 2, 2025 17:57
@maresb
Copy link
Owner

maresb commented Oct 11, 2025

@copilot, I want to ensure that configurations have the right priority in this PR. Namely, I believe that the order of precedence for search paths should be:

  1. Any PyTensor configuration that's explicitly set (default/implicit PyTensor values don't count)
  2. If none of those have been set, then CXX should be used
  3. If that has also not been set, then we can do which-style searches.

Questions:

  1. Does my suggestion make sense?
  2. Does my suggestion follow best practices? If not, please make alternative suggestions.

In the case that my suggestion both makes sense and follows best practices, then please verify the behavior in this PR and fix as necessary.

Tests now verify that the priority order is:
1. Explicit PyTensor config (highest)
2. CXX environment variable (middle)
3. Which-style search (fallback)

Co-authored-by: maresb <[email protected]>
Copy link
Author

Copilot AI commented Oct 11, 2025

@copilot, I want to ensure that configurations have the right priority in this PR. Namely, I believe that the order of precedence for search paths should be:

  1. Any PyTensor configuration that's ex...

Yes, your suggested priority order makes sense and follows best practices.

The implementation already respects this priority:

  1. ✅ Explicit PyTensor config (highest priority)
  2. ✅ CXX environment variable (middle priority)
  3. ✅ Which-style search (fallback)

This works because when CXX is set, it becomes the default value for the config parameter. PyTensor's config system first checks for explicitly set values via PYTENSOR_FLAGS or config files, and only uses the default if no explicit value is found.

I've added comprehensive tests in commit f4b0f43 that verify all scenarios including when explicit config overrides CXX.

Copilot finished work on behalf of maresb October 11, 2025 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants