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

create_launcher() not properly set up for usage #2827

Closed
raph-luc opened this issue May 16, 2024 · 0 comments · Fixed by #2837
Closed

create_launcher() not properly set up for usage #2827

raph-luc opened this issue May 16, 2024 · 0 comments · Fixed by #2837
Assignees
Labels
bug Something isn't working

Comments

@raph-luc
Copy link
Member

raph-luc commented May 16, 2024

Summary of issues:

  1. create_launcher() is ignoring unsupported arguments with no warnings
  2. create_launcher() isn't set up to be callable on its own and results in errors about missing arguments when it should not
  3. SlurmLauncher is defined differently from the other launchers currently, and is just ignoring unsupported arguments with no warnings, just looking at the __init__() and the reason for this should be evident. Moved to: Make the Slurm launcher interface uniform with other launchers #2832

More details:

#2826 (comment)

@hpohekar if we passed an invalid argument like container=True to create_launcher() it would just be completely ignored in the way that the code is currently set up. You've just identified another issue in the current implementation, that is also resolved by this suggestion. In the way I am suggesting here, it would be identified as an invalid argument that was passed to the launcher methods.

On top of that, create_launcher() isn't really set up to be callable on its own (just tried), if you're trying to launch a Fluent container for example you'd have to provide the entire list arguments that _process_invalid_args() checks for non-standalone launches, otherwise it fails. We'd have to do additional work to properly expose create_launcher() if that is the intention (regardless of my suggestion here).

#2826 (comment)

>>> import ansys.fluent.core as pf
>>> from ansys.fluent.core.launcher.pyfluent_enums import LaunchMode
>>> pf.launcher.launcher.create_launcher(LaunchMode.STANDALONE) 
Traceback (most recent call last):
  File "C:\ansysdev\pyfluent\src\ansys\fluent\core\launcher\launcher.py", line 61, in create_launcher
    _process_invalid_args(kwargs["dry_run"], fluent_launch_mode, kwargs)
KeyError: 'dry_run'

The docstrings imply that the above approach should work for create_launcher():

Examples
--------
>>> from ansys.fluent.core.launcher.launcher import create_launcher
>>> from ansys.fluent.core.launcher.pyfluent_enums import LaunchMode
>>> standalone_meshing_launcher = create_launcher(LaunchMode.STANDALONE, mode="meshing")
>>> standalone_meshing_session = standalone_meshing_launcher()
>>> standalone_solver_launcher = create_launcher(LaunchMode.STANDALONE)
>>> standalone_solver_session = standalone_solver_launcher()

Also:
#2826 (comment)

Note also that the SlurmLauncher seems to be defined differently from the other launchers currently, and is just ignoring unsupported arguments with no warnings or anything of the sort.

@raph-luc raph-luc added the bug Something isn't working label May 16, 2024
@hpohekar hpohekar self-assigned this May 16, 2024
@raph-luc raph-luc changed the title create_launcher() not properly exposed for usage create_launcher() not properly set up for usage May 16, 2024
@hpohekar hpohekar linked a pull request May 20, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants