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

parsl vs typeguard 4 doesn't work #3017

Closed
parantapa opened this issue Jan 9, 2024 · 8 comments · Fixed by #3044
Closed

parsl vs typeguard 4 doesn't work #3017

parantapa opened this issue Jan 9, 2024 · 8 comments · Fixed by #3044
Labels

Comments

@parantapa
Copy link

Describe the bug
multiprocessing.Queue is used as a generic type in "monitoring/monitoring.py"; example case; see [1]
multiprocessing.Queue is not a "type" (at-least in Python 3.11.7)
Quoting pydoc:
"multiprocessing.Queue = Queue(maxsize=0) method of multiprocessing.context.DefaultContext instance
Returns a queue object"
This is causing code that uses MonitoringHub to fail.

[1]: See:

comm_q: Queue[Union[Tuple[int, int], str]]

To Reproduce

  1. Setup Parsl 2024.1.8 with Python 3.11.7
  2. Run the following test script:
> cat test.py                                                                                                                                                                                                                                      (episim37)
import parsl
from parsl.monitoring.monitoring import MonitoringHub
from parsl.config import Config
from parsl.executors import HighThroughputExecutor
from parsl.addresses import address_by_hostname

config = Config(
   executors=[
       HighThroughputExecutor(
           label="local_htex",
           cores_per_worker=1,
           max_workers=4,
           address=address_by_hostname(),
       )
   ],
   monitoring=MonitoringHub(
       hub_address=address_by_hostname(),
       hub_port=55055,
       monitoring_debug=False,
       resource_monitoring_interval=10,
   ),
   strategy='none'
)

parsl.load(config)
  1. Run script
python test.py
  1. See error
Traceback (most recent call last):
  File "/home/parantapa/test.py", line 25, in <module>
    parsl.load(config)
  File "/home/parantapa/miniconda3/envs/episim37/lib/python3.11/site-packages/parsl/dataflow/dflow.py", line 1428, in load
    cls._dfk = DataFlowKernel(config)
               ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/parantapa/miniconda3/envs/episim37/lib/python3.11/site-packages/parsl/dataflow/dflow.py", line 116, in __init__
    self.hub_interchange_port = self.monitoring.start(self.run_id, self.run_dir)
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/parantapa/miniconda3/envs/episim37/lib/python3.11/site-packages/parsl/monitoring/monitoring.py", line 171, in start
    comm_q: Queue[Union[Tuple[int, int], str]]
            ~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: 'method' object is not subscriptable

Expected behavior
Code to not crash

Environment

  • OS: ArchLinux
  • Python version: 3.11.7
  • Parsl version: 2024.1.8

Distributed Environment

  • Where are you running the Parsl script from ? [e.g. Laptop/Workstation, Login node, Compute node]
    Laptop

  • Where do you need the workers to run ? [e.g. Same as Parsl script, Compute nodes, Cloud nodes]
    Laptop

@parantapa parantapa added the bug label Jan 9, 2024
@benclifford
Copy link
Collaborator

can you look which version of typeguard you have installed?

@parantapa
Copy link
Author

pip freeze | grep typeguard
typeguard==4.1.5

@benclifford
Copy link
Collaborator

ok, that shouldn't be happening - because parsl install requirements should be constrained by pip to this (in requirements.txt): typeguard>=2.10,<3

@parantapa
Copy link
Author

I see. I have another package installed that needs a more recent version of typeguard. I guess that upgraded the typeguard requirement.

However, is there a chance that this can be fixed (instead of constraining typeguard?)

@benclifford
Copy link
Collaborator

it's been in the works for a while - there are some API changes that are awkward if maintaining support for typeguard 2, which I'd like to also keep supporting for a while

@benclifford
Copy link
Collaborator

crossref #2812 which fixes v4 but breaks v2

@benclifford benclifford changed the title Parsl crashes due to incorrect use of type annotations on Python 3.11 parsl vs typeguard 4 doesn't work Jan 10, 2024
benclifford added a commit that referenced this issue Jan 31, 2024
This PR loosens some type checking to accomodate both typeguard 2.x and 4.x
as large parsl users (eg. LSST DESC) still package version 2.x but other users
want the latest - 4.x

In typeguard 4.x, local variables are instrumented for type-checking, which
is incompatible with using multiprocessing.Queue: multiprocessing.Queue is
not a type as far as typeguard is concerned, so a single use of that, in
MonitoringHub, is commented out by this PR.

Typeguard 2.x and 4.x have different type error classes, so explicit type
checking of exception classes cannot happen in tests as the typeguard 4.x
TypeCheckError class does not exist when typeguard 2.x is installed. This
PR makes that type-check of type-checking errors happen using string
matching of the name of the type-check error class in
test_bash_apps/test_stdout.py

See for example issue #3017

This PR is ugly but attempts to be pragmatic about user packaging needs.
benclifford added a commit that referenced this issue Jan 31, 2024
This PR loosens some type checking to accomodate both typeguard 2.x and 4.x as large parsl users (eg. LSST DESC) still package version 2.x but other users want the latest - 4.x

In typeguard 4.x, local variables are instrumented for type-checking, which is incompatible with using multiprocessing.Queue: multiprocessing.Queue is not a type as far as typeguard is concerned, so a single use of that, in MonitoringHub, is commented out by this PR.

Typeguard 2.x and 4.x have different type error classes, so explicit type checking of exception classes cannot happen in tests as the typeguard 4.x TypeCheckError class does not exist when typeguard 2.x is installed. This PR makes that type-check of type-checking errors happen using string matching of the name of the type-check error class in test_bash_apps/test_stdout.py

See for example issue #3017

This PR is ugly but attempts to be pragmatic about user packaging needs.
@benclifford
Copy link
Collaborator

@parantapa you might try Parsl now that #3044 has been merged

@parantapa
Copy link
Author

@benclifford thanks for the heads up! this is really helpful.

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

Successfully merging a pull request may close this issue.

2 participants