[S549767] Start Fluent from PyFluent #7

mkundu1 · 2022-01-14T12:06:20Z

Add a launch_fluent method which will launch Fluent and return a Session instance. Currently launch_fluent supports only few common arguments to launch Fluent locally which are validated and converted to Fluent launch arguments using static information from launch_options.yaml file. Minor change is required in Fluent side to cleanly exit Fluent from PyFluent which has added in PR 290947.

PyFluent should cover some of the launch_mapdl arguments (#5) - will look into these separately. Some of these like start_timeout should be handled in the client side.

h-krishnan · 2022-01-14T12:25:32Z

ansys/fluent/launcher/launcher_options.yaml

+processor_count:
+  type: int
+  fluent_format: " -t{}"
+journal_filename:


Note that Fluent supports multiple specifications of -i <joufile>.

@HKrishnan-ansys will add different type for journal_filename to handle it

Unless you pass the -cloud option... :-) Learned that this week.

h-krishnan · 2022-01-14T12:26:21Z

ansys/fluent/session.py

        if self.__channel:
            self.__channel.close()
+        Path(self.server_info_filepath).unlink(missing_ok=True)


This file can be deleted as soon as the connection is done, right?

Right, thanks

h-krishnan · 2022-01-14T12:45:11Z

ansys/fluent/launcher/launcher.py

+                argval = fluent_values[i]
+            launch_string += v['fluent_format'].replace('{}', str(argval))
+    server_info_filepath = get_server_info_filepath()
+    launch_string += f' -sifile="{server_info_filepath}"'


@mkundu1 Sorry to be finicky here but I am wondering if it is better to remove the file in this place in a try-finally block (you may want to put the unlink call also in a try-except block).

@HKrishnan-ansys No problem, yeah that's better.

h-krishnan

Looks good.

akaszynski · 2022-01-14T17:27:47Z

ansys/fluent/launcher/launcher.py

+FLUENT_MAJOR_VERSION = '22'
+FLUENT_MINOR_VERSION = '2'
+LAUNCH_FLUENT_TIMEOUT = 100


These all will have to be user configurable.

Yes, and is there a reason to split them apart into two variables? Why not just FLUENT_VERSION='22.2'

Changed to FLUENT_VERSION='22.2'

akaszynski · 2022-01-14T17:30:01Z

ansys/fluent/launcher/launcher.py

+    version = None,
+    precision = None,
+    processor_count = None,
+    journal_filename = None


Nitpick: there should be no whitespace between = for keyword args.

Yeah, is that part of the formatting guidelines already?

akaszynski · 2022-01-14T17:30:38Z

ansys/fluent/launcher/launcher.py

+
+    Returns
+    -------
+    An instance of ansys.fluent.session.Session.


Should follow numpy doc Returns style:

Suggested change

An instance of ansys.fluent.session.Session.

ansys.fluent.session.Session

Fluent session.

Changed docstring to follow numpydoc style

akaszynski · 2022-01-14T17:31:01Z

ansys/fluent/launcher/launcher.py

+    """Start Fluent locally in server mode.
+    Parameters


There has to be whitespace between short description and the Parameters section.

Suggested change

"""Start Fluent locally in server mode.

Parameters

"""Start Fluent locally in server mode.

Parameters

akaszynski · 2022-01-14T17:32:20Z

ansys/fluent/launcher/launcher.py

+        Whether to use the '2d' or '3d' version of Fluent. Default is '3d'.
+
+    precision : str, optional
+        Whether to use the single-precision or double-precision version of Fluent. Default is double-precision.


Be sure to escape literals.

Also, consider that the PEP8 standard for docstring length is 72:

PEP8 has some maximum line length guidelines, starting with “Limit all lines to a maximum of 79 characters” but “for flowing long blocks of text with fewer structural restrictions (docstrings or comments), the line length should be limited to 72 characters.”

I'm ok if we go to 79, but any longer and it won't be rendered correctly in some "old school" editors.

Suggested change

Whether to use the single-precision or double-precision version of Fluent. Default is double-precision.

Whether to use the single-precision or double-precision version of Fluent. Default is ``"double-precision"``.

Agree we should stick to PEP8 here.

akaszynski · 2022-01-14T17:33:04Z

ansys/fluent/launcher/launcher.py

+                    print(f'Default value {argval} is chosen for {k} as the passed value '
+                          f'{old_argval} is outside allowed_values {allowed_values}.')


Don't print, either warn or raise. This won't show up in unit testing and users could potentially be passing invalid args.

akaszynski · 2022-01-14T17:33:52Z

ansys/fluent/launcher/launcher.py

+        if argval is None and v.get('required', None) == True:
+            argval = default
+        if argval is not None:
+            allowed_values = v.get('allowed_values', None)


Note that None is redundant for get. If a key does not exist, None is returned by default

Yeah, that would clean it up.

akaszynski · 2022-01-14T17:34:49Z

ansys/fluent/launcher/launcher.py

+    for k, v in all_options.items():
+        argval = argvals.get(k, None)
+        default = v.get('default', None)
+        if argval is None and v.get('required', None) == True:


I'm sure you know, but: https://stackoverflow.com/questions/27276610/boolean-identity-true-vs-is-true

Suggested change

if argval is None and v.get('required', None) == True:

if argval is None and v.get('required') is True:

akaszynski · 2022-01-14T17:35:31Z

ansys/fluent/launcher/launcher.py

+        launch_string += f' -sifile="{server_info_filepath}"'
+        if not os.getenv('PYFLUENT_SHOW_SERVER_GUI'):
+            launch_string += ' -hidden'
+        print(f'Launching Fluent with cmd: {launch_string}')


Should be using logging here, avoid printing always.

https://dev.docs.pyansys.com/guidelines/logging.html

I've added a basic logger class to log to console - will expand that to include log files, etc. in future.

akaszynski · 2022-01-14T17:35:40Z

ansys/fluent/launcher/launcher.py

+        while True:
+            if Path(server_info_filepath).stat().st_mtime > sifile_last_mtime:
+                time.sleep(1)
+                print('\nFluent process is successfully launched.')


See logging note.

akaszynski · 2022-01-14T17:36:16Z

ansys/fluent/session.py

+def parse_server_info_file(filename: str):
+    with open(filename, "rb") as f:
+        lines = f.readlines()
+    return (lines[0].strip(), lines[1].strip())


Unnecessary tuple:

Suggested change

return (lines[0].strip(), lines[1].strip())

return lines[0].strip(), lines[1].strip()

akaszynski · 2022-01-14T17:39:21Z

ansys/fluent/session.py

        self.__channel = grpc.insecure_channel(address)
        datamodel_stub = DataModelGrpcModule.DataModelStub(self.__channel)
        self.service = DatamodelService(datamodel_stub, password)
        self.tui = Session.tui(self.service)

-    def stop(self):
+    def close(self):


Provide documentation here please.

akaszynski · 2022-01-14T17:39:43Z

README.rst

@@ -40,5 +36,5 @@ In Python (client-side):
  session.tui.display.objects.contour['contour-1'].color_map.size()
  session.tui.display.objects.contour['contour-1'].rename('my-contour')
  del session.tui.display.objects.contour['my-contour']
-  session.stop()
+  session.close()


Curious, and it's up to you, but should we have close or exit. Close implies you're closing the connection, while exit implies you're shutting down the fluent instance. I can't tell (as a user/onlooker) what close is doing since there's no docstring.

I think it means close the connection right? i.e. I can connect again later?

I've changed this to session.exit. session.exit closes the connection and exit Fluent. At present we cannot re-use the session object - will check if we should do it. Perhaps, we shall recommend users to use session within a with context.

akaszynski · 2022-01-14T17:41:26Z

ansys/fluent/launcher/launcher.py

+        print(f'Launching Fluent with cmd: {launch_string}')
+        sifile_last_mtime = Path(server_info_filepath).stat().st_mtime
+        kwargs = get_subprocess_kwargs_for_detached_process()
+        subprocess.Popen(launch_string, **kwargs)


Consider piping stdout/stderr to a listener thread that writes to a logger. That way users can determine why fluent fails to launch. I've tried a similar implementation for MAPDL, but there's an issue with stdout/stderr piping otherwise we would have implemented it; I hope you don't encounter the same issue here.

Thanks, I'll check this.

akaszynski · 2022-01-14T17:41:50Z

ansys/fluent/launcher/launcher.py

+                print('\nFluent process is successfully launched.')
+                break
+            if counter == 0:
+                print('\nThe launch process has been timed out.')


Warn or raise.

akaszynski · 2022-01-14T17:41:59Z

ansys/fluent/launcher/launcher.py

+                break
+            time.sleep(1)
+            counter -= 1
+            print(f'Waiting for Fluent to launch...{counter:2} seconds remaining', end='\r')


dnwillia-work · 2022-01-14T20:56:04Z

README.rst

@@ -40,5 +36,5 @@ In Python (client-side):
  session.tui.display.objects.contour['contour-1'].color_map.size()
  session.tui.display.objects.contour['contour-1'].rename('my-contour')
  del session.tui.display.objects.contour['my-contour']
-  session.stop()
+  session.close()


I think it means close the connection right? i.e. I can connect again later?

dnwillia-work

Mainak, please incorporate the suggestions from Alex.

dnwillia-work · 2022-01-14T21:12:09Z

ansys/fluent/launcher/launcher.py

+    return str(exe_path)
+
+def get_server_info_filepath():
+    server_info_dir = os.getenv('SERVER_INFO_DIR')


What sets up this environment variable? I guess we should also add keyword options to pass the IP/hostname and port to the lauch_fluent function?

Right, we'll need to change launcher.py to pass the host/port info for docker launch. Current it assumes Fluent is launched from a local machine. SERVER_INFO_DIR is documented feature in Fluent to specify the location of the server-info file.

@mkundu1 Could we use temp folder instead of Path.cwd() if SERVER_INFO_DIR is not specified?

Yes, we can use the temp folder, will change it in the next PR.

mkundu1 · 2022-01-17T08:45:31Z

Thanks @akaszynski and @dnwillia-ansys for your valuable suggestions. I've incorporated most of them. There are a couple remaining, will look into those in subsequent PRs.

h-krishnan · 2022-01-17T08:45:34Z

ansys/fluent/launcher/launcher.py

+    journal_filename : str, optional
+        Read the specified journal file.
+
+    start_timeout : float, optional


@mkundu1 Is this float or int?

Will fix in next PR

[S549767] Start Fluent from PyFluent

39759ee

h-krishnan reviewed Jan 14, 2022

View reviewed changes

[S549767] Start Fluent from PyFluent

bcd6b1b

h-krishnan reviewed Jan 14, 2022

View reviewed changes

[S549767] Start Fluent from PyFluent

66e84b7

mkundu1 requested review from ajain-work, dnwillia-work, seanpearsonuk and h-krishnan January 14, 2022 13:21

seanpearsonuk approved these changes Jan 14, 2022

View reviewed changes

h-krishnan approved these changes Jan 14, 2022

View reviewed changes

akaszynski reviewed Jan 14, 2022

View reviewed changes

dnwillia-work approved these changes Jan 14, 2022

View reviewed changes

dnwillia-work requested changes Jan 14, 2022

View reviewed changes

dnwillia-work reviewed Jan 14, 2022

View reviewed changes

mkundu1 added 6 commits January 17, 2022 11:02

[S549767] Linting

07bf950

[S549767] Start Fluent from PyFluent

68469c2

[S549767] Start Fluent from PyFluent

ea84faa

[S549767] Start Fluent from PyFluent

065979d

[S549767] Start Fluent from PyFluent

602a6c5

[S549767] Logging

b374706

h-krishnan reviewed Jan 17, 2022

View reviewed changes

mkundu1 mentioned this pull request Jan 17, 2022

Logging Fluent launch errors #9

Closed

mkundu1 merged commit d740850 into main Jan 17, 2022

mkundu1 deleted the mkundu/s549767/start-fluent branch March 4, 2022 04:41

mkundu1 mentioned this pull request Feb 22, 2024

Error when running PyFluent in HPC/Slurm environment #2351

Closed

2 tasks

tejalprabhu01 mentioned this pull request Oct 1, 2024

HDF-5 error while writing case file #3333

Closed

2 tasks

	An instance of ansys.fluent.session.Session.
	ansys.fluent.session.Session
	Fluent session.

	Whether to use the single-precision or double-precision version of Fluent. Default is double-precision.
	Whether to use the single-precision or double-precision version of Fluent. Default is ``"double-precision"``.

		print(f'Default value {argval} is chosen for {k} as the passed value '
		f'{old_argval} is outside allowed_values {allowed_values}.')

	if argval is None and v.get('required', None) == True:
	if argval is None and v.get('required') is True:

	return (lines[0].strip(), lines[1].strip())
	return lines[0].strip(), lines[1].strip()

[S549767] Start Fluent from PyFluent #7

[S549767] Start Fluent from PyFluent #7

Conversation

mkundu1 commented Jan 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h-krishnan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnwillia-work Jan 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkundu1 Jan 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnwillia-work Jan 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkundu1 Jan 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnwillia-work left a comment

Choose a reason for hiding this comment

dnwillia-work Jan 14, 2022 • edited Loading

Choose a reason for hiding this comment

mkundu1 Jan 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkundu1 commented Jan 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkundu1 commented Jan 14, 2022 •

edited

Loading

dnwillia-work Jan 14, 2022 •

edited

Loading

mkundu1 Jan 17, 2022 •

edited

Loading

dnwillia-work Jan 14, 2022 •

edited

Loading

mkundu1 Jan 17, 2022 •

edited

Loading

dnwillia-work Jan 14, 2022 •

edited

Loading

mkundu1 Jan 17, 2022 •

edited

Loading