Ensure that gcc path is only added once to DLL search path #678

maresb · 2024-03-22T16:24:01Z

Description

This is an attempt to solve

tests\distributions\test_transform.py:347: in check_vectortransform_elementwise_logp
    self.check_transform_elementwise_logp(model, vector_transform=True)
tests\distributions\test_transform.py:318: in check_transform_elementwise_logp
    test_array_untransf = x_val_untransf.eval({x_val_transf: test_array_transf})
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\graph\basic.py:616: in eval
    self._fn_cache[inputs] = function(inputs, self)
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\compile\function\__init__.py:315: in function
    fn = pfunc(
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\compile\function\pfunc.py:469: in pfunc
    return orig_function(
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\compile\function\types.py:1762: in orig_function
    fn = m.create(defaults)
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\compile\function\types.py:1654: in create
    _fn, _i, _o = self.linker.make_thunk(
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\link\basic.py:245: in make_thunk
    return self.make_all(
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\link\vm.py:1244: in make_all
    raise_with_op(fgraph, node)
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\link\utils.py:531: in raise_with_op
    raise exc_value.with_traceback(exc_trace)
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\link\vm.py:1235: in make_all
    node.op.make_thunk(node, storage_map, compute_map, [], impl=impl)
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\link\c\op.py:119: in make_thunk
    return self.make_c_thunk(node, storage_map, compute_map, no_recycling)
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\link\c\op.py:84: in make_c_thunk
    outputs = cl.make_thunk(
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\link\c\basic.py:1209: in make_thunk
    cthunk, module, in_storage, out_storage, error_storage = self.__compile__(
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\link\c\basic.py:1129: in __compile__
    thunk, module = self.cthunk_factory(
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\link\c\basic.py:1653: in cthunk_factory
    module = cache.module_from_key(key=key, lnk=self)
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\link\c\cmodule.py:1231: in module_from_key
    module = lnk.compile_cmodule(location)
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\link\c\basic.py:1552: in compile_cmodule
    module = c_compiler.compile_str(
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\link\c\cmodule.py:2652: in compile_str
    return dlimport(lib_filename)
C:\Users\runneradmin\miniconda3\envs\pymc-test\Lib\site-packages\pytensor\link\c\cmodule.py:325: in dlimport
    os.add_dll_directory(os.path.dirname(gcc_path))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

path = 'C:\\Users\\runneradmin\\miniconda3\\envs\\pymc-test\\Library\\mingw-w64\\bin'

>   ???
E   FileNotFoundError: [WinError 206] The filename or extension is too long: 'C:\\Users\\runneradmin\\miniconda3\\envs\\pymc-test\\Library\\mingw-w64\\bin'

This is a wild guess at the cause since I don't have Windows, and it's appearing only in the PyMC test suite. In any case I think this is unlikely to hurt.

Related Issue

Closes #
Related to #

Checklist

Checked that the pre-commit linting/style checks pass
Included tests that prove the fix is effective or that the new feature works
Added necessary documentation (docstrings and/or example notebooks)
If you are a pro: each commit corresponds to a relevant logical change

Type of change

ricardoV94 · 2024-03-22T16:56:54Z

This is an attempt to solve

The suspense is killing me

maresb · 2024-03-22T17:02:51Z

The suspense is killing me

Look at the stack trace next. ;)

lucianopaz · 2024-04-02T09:28:44Z

I haven’t tested this but I personally don’t understand how this could fix the problem. Aren’t we bound to hit the same problem again if we the Windows runner has some weird and long path to gcc?
I googled a bit and found that Windows has a protocol to use more bits for the path https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry. Perhaps we could also use that prefix for the windows paths? Anyway, adding gcc only once seems perfectly fine and I like it much more than what we had before.

maresb · 2024-04-02T09:44:40Z

@lucianopaz, I'm not protecting against long paths, I'm protecting against the path being added over and over again.

lucianopaz · 2024-04-02T11:27:36Z

@lucianopaz, I'm not protecting against long paths, I'm protecting against the path being added over and over again.

Yes, I understand that, but I don’t understand why doing that prevents the error you were seeing with long path names.

maresb · 2024-04-02T15:19:07Z

My hypothesis is simply that the error message is wrong. If you look at my stack trace, the path that triggered the error has quite an ordinary length. So I am hoping that when we encounter those absurdly long Conda prefix paths that we're only triggering the limit because we add it multiple times.

Like I wrote above, I'm not sure this will solve the problem, but I suspect it might.

codecov-commenter · 2024-04-02T15:47:53Z

Codecov Report

Attention: Patch coverage is 50.00000% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 80.76%. Comparing base (f7b0a7a) to head (f727687).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #678   +/-   ##
=======================================
  Coverage   80.76%   80.76%           
=======================================
  Files         162      162           
  Lines       46694    46698    +4     
  Branches    11420    11421    +1     
=======================================
+ Hits        37711    37715    +4     
  Misses       6734     6734           
  Partials     2249     2249

Files	Coverage Δ
pytensor/link/c/cmodule.py	`56.76% <50.00%> (+0.13%)`	⬆️

pytensor/link/c/cmodule.py

HarshvirSandhu · 2024-04-04T09:32:13Z

I was working on #685 but when I run pre-commit locally, mypy fails with this message:

!!!!!!!!!
1 files unexpectedly failed.
pytensor/link/c/cmodule.py
These files did not fail before, so please check the above output for errors in {'pytensor/link/c/cmodule.py'} and fix them.

maresb · 2024-04-04T20:34:03Z

Hi @HarshvirSandhu, thanks for letting us know, but could you please provide more details? In particular, could you please provide the specific mypy error that is reported for pytensor/link/c/cmodule.py?

This is a bit strange since I'm unable to reproduce it locally, and usually these things are caught by the automated tests.

Dhruvanshu-Joshi · 2024-04-10T13:57:03Z

Hello @maresb and @HarshvirSandhu ,
I am facing this same error when mypy is run locally:

[pytensor\link\c\cmodule.py]
pytensor\link\c\cmodule.py:287: error: Unused "type: ignore" comment

pytensor/pytensor/link/c/cmodule.py

Line 287 in bc3dda0

os.add_dll_directory(os.path.dirname(gcc_path)) # type: ignore

maresb · 2024-04-11T08:07:09Z

Thanks @Dhruvanshu-Joshi for the info! It seems quite mysterious why we are now seeing different results when mypy is run locally vs in the CI. (We see that the CI checks are failing in #702 after you added cmodule.py to scripts/mypy-failing.txt because there are no errors in the CI.) Are you running mypy via pre-commit? What operating system are you using?

Dhruvanshu-Joshi · 2024-04-11T16:37:02Z

@maresb I am running mypy via pre-commit. Currently, I am using windows 11.

As reported in <pymc-devs#678 (comment)>, mypy was failing on Windows due to an unnecessary type: ignore. This fixes it by ignoring the unused-ignore so that Windows is covered.

As reported in <#678 (comment)>, mypy was failing on Windows due to an unnecessary type: ignore. This fixes it by ignoring the unused-ignore so that Windows is covered.

maresb mentioned this pull request Mar 22, 2024

Switch to setup-micromamba action pymc-devs/pymc#7213

Merged

11 tasks

ricardoV94 requested a review from lucianopaz March 22, 2024 19:06

ricardoV94 added bug Something isn't working C-backend labels Mar 22, 2024

maresb force-pushed the fix-gcc-dll-path branch from ffabc07 to f727687 Compare April 2, 2024 15:22

twiecki reviewed Apr 3, 2024

View reviewed changes

pytensor/link/c/cmodule.py Outdated Show resolved Hide resolved

Ensure that gcc path is only added once to DLL search path

36faf21

maresb force-pushed the fix-gcc-dll-path branch from f727687 to 36faf21 Compare April 3, 2024 09:33

twiecki approved these changes Apr 3, 2024

View reviewed changes

lucianopaz approved these changes Apr 3, 2024

View reviewed changes

twiecki merged commit 76a6c2e into pymc-devs:main Apr 3, 2024

maresb deleted the fix-gcc-dll-path branch April 3, 2024 15:29

Dhruvanshu-Joshi mentioned this pull request Apr 10, 2024

Add matrix_transpose and .mT helpers #702

Merged

11 tasks

This was referenced Apr 12, 2024

Fix mypy problem on Windows due to platform differences #706

Merged

Use add_dll_directory as a context manager #708

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Ensure that gcc path is only added once to DLL search path #678

Ensure that gcc path is only added once to DLL search path #678

Uh oh!

maresb commented Mar 22, 2024

Uh oh!

ricardoV94 commented Mar 22, 2024

Uh oh!

maresb commented Mar 22, 2024 •

edited

Loading

Uh oh!

lucianopaz commented Apr 2, 2024

Uh oh!

maresb commented Apr 2, 2024

Uh oh!

lucianopaz commented Apr 2, 2024

Uh oh!

maresb commented Apr 2, 2024

Uh oh!

codecov-commenter commented Apr 2, 2024

Uh oh!

Uh oh!

HarshvirSandhu commented Apr 4, 2024

Uh oh!

maresb commented Apr 4, 2024

Uh oh!

Dhruvanshu-Joshi commented Apr 10, 2024

Uh oh!

maresb commented Apr 11, 2024

Uh oh!

Dhruvanshu-Joshi commented Apr 11, 2024

Uh oh!

Uh oh!

Ensure that gcc path is only added once to DLL search path #678

Ensure that gcc path is only added once to DLL search path #678

Uh oh!

Conversation

maresb commented Mar 22, 2024

Description

Related Issue

Checklist

Type of change

Uh oh!

ricardoV94 commented Mar 22, 2024

Uh oh!

maresb commented Mar 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lucianopaz commented Apr 2, 2024

Uh oh!

maresb commented Apr 2, 2024

Uh oh!

lucianopaz commented Apr 2, 2024

Uh oh!

maresb commented Apr 2, 2024

Uh oh!

codecov-commenter commented Apr 2, 2024

Codecov Report

Uh oh!

Uh oh!

HarshvirSandhu commented Apr 4, 2024

Uh oh!

maresb commented Apr 4, 2024

Uh oh!

Dhruvanshu-Joshi commented Apr 10, 2024

Uh oh!

maresb commented Apr 11, 2024

Uh oh!

Dhruvanshu-Joshi commented Apr 11, 2024

Uh oh!

Uh oh!

maresb commented Mar 22, 2024 •

edited

Loading