materialsproject / fireworks

The Fireworks Workflow Management Repo.
https://materialsproject.github.io/fireworks
Other
351 stars 184 forks source link

qlaunch not working on a fresh install #483

Closed jmmshn closed 2 years ago

jmmshn commented 2 years ago

qlaunch not working on a fresh install

It appears a pytest import was added in the path of USER_PACKAGES

https://github.com/materialsproject/fireworks/blob/51aa296dff9cd0411ad9165973dd4bc5c4a5c7aa/fireworks/fw_config.py#L20

And the part of the code that is meant to deal with the exception has a call to traceback.print_exc(ex) which is treating the ex exception as the limit parameter.

https://github.com/materialsproject/fireworks/blob/635fca928777610e5933a4f197c0be024aaa82ef/fireworks/utilities/fw_serializers.py#L362

Code for print_exc:

Signature: traceback.print_exc(limit=None, file=None, chain=True)
Source:   
def print_exc(limit=None, file=None, chain=True):
    """Shorthand for 'print_exception(*sys.exc_info(), limit, file)'."""
    print_exception(*sys.exc_info(), limit=limit, file=file, chain=chain)
File:      ~/miniconda3/envs/mp/lib/python3.9/traceback.py
Type:      function

Full output of the error

/g/g20/shen9/repos/mp/fireworks/fireworks/utilities/fw_serializers.py:361: UserWarning: None in fireworks.utilities.tests.test_visualize cannot be loaded because of No module named 'pytest'. Skipping..
  warnings.warn(f"{m_object} in {mod_name} cannot be loaded because of {str(ex)}. Skipping..")
Traceback (most recent call last):
  File "/g/g20/shen9/repos/mp/fireworks/fireworks/utilities/fw_serializers.py", line 354, in load_object
    m_module = importlib.import_module(mod_name)
  File "/g/g20/shen9/.conda/envs/mp2/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "/g/g20/shen9/repos/mp/fireworks/fireworks/utilities/tests/test_visualize.py", line 1, in <module>
    import pytest
ModuleNotFoundError: No module named 'pytest'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/g/g20/shen9/.conda/envs/mp2/bin/qlaunch", line 33, in <module>
    sys.exit(load_entry_point('FireWorks', 'console_scripts', 'qlaunch')())
  File "/g/g20/shen9/repos/mp/fireworks/fireworks/scripts/qlaunch_run.py", line 272, in qlaunch
    do_launch(args)
  File "/g/g20/shen9/repos/mp/fireworks/fireworks/scripts/qlaunch_run.py", line 61, in do_launch
    queueadapter = load_object_from_file(args.queueadapter_file)
  File "/g/g20/shen9/repos/mp/fireworks/fireworks/utilities/fw_serializers.py", line 397, in load_object_from_file
    return load_object(reconstitute_dates(dct))
  File "/g/g20/shen9/repos/mp/fireworks/fireworks/utilities/fw_serializers.py", line 362, in load_object
    traceback.print_exc(ex)
  File "/g/g20/shen9/.conda/envs/mp2/lib/python3.9/traceback.py", line 163, in print_exc
    print_exception(*sys.exc_info(), limit=limit, file=file, chain=chain)
  File "/g/g20/shen9/.conda/envs/mp2/lib/python3.9/traceback.py", line 103, in print_exception
    for line in TracebackException(
  File "/g/g20/shen9/.conda/envs/mp2/lib/python3.9/traceback.py", line 517, in __init__
    self.stack = StackSummary.extract(
  File "/g/g20/shen9/.conda/envs/mp2/lib/python3.9/traceback.py", line 340, in extract
    if limit >= 0:
TypeError: '>=' not supported between instances of 'ModuleNotFoundError' and 'int'
janosh commented 2 years ago

Oops, this is my fault. I added the pytest import in https://github.com/materialsproject/fireworks/pull/478/commits/f0b7a4779f7fdb803f3b6dc873156088c111b642. At the time, I wasn't aware of fireworks deserialization machinery and what consequences this import would have.

Though I still don't understand why searching test files for load_objects is even necessary. Surely the things people might want to hydrate are all in the fireworks package itself, not its tests? So I removed "fireworks.utilities.tests" from USER_PACKAGES in https://github.com/materialsproject/fireworks/pull/480/commits/e42fab166c91a3af41d5e9c2cba22f21a5ba0b06. I asked in https://github.com/materialsproject/fireworks/pull/480#discussion_r799779211 if this is ok to do and it appears so since @computron merged.

jmmshn commented 2 years ago

ok, cool so the big problem is solved! Thanks! @janosh

janosh commented 2 years ago

No one seems to know why "fireworks.utilities.tests" was there in the first place, i.e. what the use case for serializing test objects might have been. But changing stuff without knowing leaves a bad feeling. Occurs to me I didn't ask @mkhorton who liked this PR. Any insight you could give?

Andrew-S-Rosen commented 2 years ago

@computron, whenever you have a moment, would you mind making a new version? qlaunch doesn't work with 2.0.2 but is fixed in main.

jmmshn commented 2 years ago

@computron @mkhorton can we please merge and release this?

mkhorton commented 2 years ago

I'm not a maintainer here, apologies (would if I could!)

computron commented 2 years ago

hey really sorry I didn't see this, merging now!

Andrew-S-Rosen commented 2 years ago

All good, thanks for handling @computron! :)