ansys / pyfluent

Pythonic interface to Ansys Fluent
https://fluent.docs.pyansys.com
MIT License
242 stars 41 forks source link

No tests for events service #3023

Open seanpearsonuk opened 1 week ago

seanpearsonuk commented 1 week ago

It looks like we have no tests for solver.events(?)

seanpearsonuk commented 1 week ago

Code synopsis

>>> import ansys.fluent.core as pf
>>> solver = pf.launch_fluent()
>>> events = solver.events
>>> type(events)
<class 'ansys.fluent.core.streaming_services.events_streaming.EventsManager'>
>>> dir(events)
['__class__', '__delattr__', '__dict__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattribute__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__le__', '__lt__', '__module__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', '__weakref__', '_fluent_error_state', '_id', '_lock', '_prepare', '_process_streaming', '_service_callback_id', '_service_callbacks', '_service_id', '_session_id', '_stream_begin_method', '_stream_thread', '_streaming', '_streaming_service', '_target', 'is_streaming', 'register_callback', 'start', 'stop', 'unregister_callback']
>>> help(events.register_callback)
Help on method register_callback in module ansys.fluent.core.streaming_services.events_streaming:

register_callback(event_name: Union[ansys.fluent.core.streaming_services.events_streaming.Event, str], callback: Callable, *args, **kwargs) method of ansys.fluent.core.streaming_services.events_streaming.EventsManager instance
    Register the callback.

    Parameters
    ----------
    event_name : Event or str
        Event name to register the callback to.
    callback : Callable
        Callback to register.
    args : Any
        Arguments.
    kwargs : Any
        Keyword arguments.

    Returns
    -------
    str
        Registered callback ID.

    Raises
    ------
    InvalidArgument
        If event name is not valid.

>>> help(pf.streaming_services.events_streaming.Event)
Help on class Event in module ansys.fluent.core.streaming_services.events_streaming:

class Event(enum.Enum)
 |  Event(value, names=None, *, module=None, qualname=None, type=None, start=1)
 |
 |  Enumerates over supported server (Fluent) events.
 |
 |  Method resolution order:
 |      Event
 |      enum.Enum
 |      builtins.object
 |
 |  Data and other attributes defined here:
 |
 |  ABOUT_TO_INITIALIZE_SOLUTION = <Event.ABOUT_TO_INITIALIZE_SOLUTION: 'A...
 |
 |  ABOUT_TO_LOAD_CASE = <Event.ABOUT_TO_LOAD_CASE: 'AboutToReadCaseEvent'...
 |
 |  ABOUT_TO_LOAD_DATA = <Event.ABOUT_TO_LOAD_DATA: 'AboutToReadDataEvent'...
 |
 |  CALCULATIONS_ENDED = <Event.CALCULATIONS_ENDED: 'CalculationsEndedEven...
 |
 |  CALCULATIONS_PAUSED = <Event.CALCULATIONS_PAUSED: 'CalculationsPausedE...
 |
 |  CALCULATIONS_RESUMED = <Event.CALCULATIONS_RESUMED: 'CalculationsResum...
 |
 |  CALCULATIONS_STARTED = <Event.CALCULATIONS_STARTED: 'CalculationsStart...
 |
 |  CASE_LOADED = <Event.CASE_LOADED: 'CaseReadEvent'>
 |
 |  DATA_LOADED = <Event.DATA_LOADED: 'DataReadEvent'>
 |
 |  FATAL_ERROR = <Event.FATAL_ERROR: 'ErrorEvent'>
 |
 |  ITERATION_ENDED = <Event.ITERATION_ENDED: 'IterationEndedEvent'>
 |
 |  ITERATION_STARTED = <Event.ITERATION_STARTED: 'IterationStartedEvent'>
 |
 |  PROGRESS_UPDATED = <Event.PROGRESS_UPDATED: 'ProgressEvent'>
 |
 |  REPORT_DEFINITION_UPDATED = <Event.REPORT_DEFINITION_UPDATED: 'ReportD...
 |
 |  REPORT_PLOT_SET_UPDATED = <Event.REPORT_PLOT_SET_UPDATED: 'PlotSetChan...
 |
 |  RESIDUAL_PLOT_UPDATED = <Event.RESIDUAL_PLOT_UPDATED: 'ResidualPlotCha...
 |
 |  SETTINGS_CLEARED = <Event.SETTINGS_CLEARED: 'ClearSettingsDoneEvent'>
 |
 |  SOLUTION_INITIALIZED = <Event.SOLUTION_INITIALIZED: 'InitializedEvent'...
 |
 |  SOLUTION_PAUSED = <Event.SOLUTION_PAUSED: 'AutoPauseEvent'>
 |
 |  SOLVER_TIME_ESTIMATE_UPDATED = <Event.SOLVER_TIME_ESTIMATE_UPDATED: 'S...
 |
 |  TIMESTEP_ENDED = <Event.TIMESTEP_ENDED: 'TimestepEndedEvent'>
 |
 |  TIMESTEP_STARTED = <Event.TIMESTEP_STARTED: 'TimestepStartedEvent'>
 |
 |  ----------------------------------------------------------------------
 |  Data descriptors inherited from enum.Enum:
 |
 |  name
 |      The name of the Enum member.
 |
 |  value
 |      The value of the Enum member.
 |
 |  ----------------------------------------------------------------------
 |  Readonly properties inherited from enum.EnumMeta:
 |
 |  __members__
 |      Returns a mapping of member name->value.
 |
 |      This mapping lists all enum members, including aliases. Note that this
 |      is a read-only view of the internal mapping.

Example from the PyFluent code:


        self.events.register_callback(Event.SOLUTION_INITIALIZED, self.monitors.refresh)
        self.events.register_callback(Event.DATA_LOADED, self.monitors.refresh)

# where refresh is defined thus:

       def refresh(self, session_id, event_info) -> None: 
           # ...

For initial, simplistic test purposes, the arguments can be ignored inside the callback. However, for the sake of more detailed testing, why is the callback being passed a session id rather than an actual session, and how does a user know what to do with a session id? Also, what is event_info - do we document what this is? To facilitate more detailed testing, each of these things need to be cleaned up and/or clarified.

EventManager is invoking callbacks thus

callback(
                            session_id=self._session_id,
                            event_info=getattr(response, event_name.value.lower()),
                        )

So, is this event_info just the event name which we are now hiding from the registration interface? We need to clean this up so that the new enum is used instead of the string values. Can we also eliminate this session_id concept? We need to document all of this as well.

@mkundu1 @prmukherj @hpohekar for discussion of these questions. @cj-hodgson for the testing part(?) - please hold off while we figure out any changes.

seanpearsonuk commented 1 week ago

I propose adding a callback_has_new_signature(default=False) to register_callback(). Then, at invocation, if callback_has_new_signature is False we make the call in the deprecated format and issue a deprecation warning. Eventually we stop supporting callback_has_new_signature=False, whereupon we deprecate callback_has_new_signature itself and eventually eliminate it.

mkundu1 commented 1 week ago

Please also consider this for PyConsole automated testing.

mkundu1 commented 1 week ago

The Event enum needs to be at ansys.fluent.core level.

seanpearsonuk commented 1 week ago

The Event enum needs to be at ansys.fluent.core level.

Yes, I'm making the change to expose it properly

seanpearsonuk commented 5 days ago

Please also consider this for PyConsole automated testing.

@cj-hodgson n.b. I have added a test in tests/test_events_manager.py for this.