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

Expose settings root implicitly. #2263

Merged
merged 18 commits into from Jan 4, 2024
Merged

Conversation

prmukherj
Copy link
Collaborator

@prmukherj prmukherj commented Nov 21, 2023

Settings root to be exposed implicitly.

For fluent versions till 23.1 there are a fixed set of settings attributes like: "file", "mesh", "setup", "solution", "results", "parametric_studies", "current_parametric_study", "parameters", "parallel", "report", and "server". For versions 23.2 and above, these attributes are directly taken from root and populated.

This removes hard coding of the attributes which were affected by changes to settings-api.

@prmukherj prmukherj linked an issue Nov 21, 2023 that may be closed by this pull request
@prmukherj prmukherj marked this pull request as ready for review December 13, 2023 08:05
Copy link
Member

@raph-luc raph-luc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: The way we are providing dict(root) here is going to be missing attributes initially, compared to before. For example, with these changes I get:

>>> session = pf.launch_fluent()
>>> dir(session)
['__bool__', '__call__', '__class__', '__delattr__', '__dict__', '__dir__', '__doc__', '__enter__', '__eq__', '__exit__', '__format__', '__ge__', '__getattr__', '__getattribute__', '__getstate__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__le__', '__lt__', '__module__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', '__weakref__', '_batch_ops_service', '_build_from_fluent_connection', '_field_data_service', '_lck', '_monitors_service', '_pim_methods', '_preferences', '_reduction_service', '_remote_file_handler', '_root', '_se_service', '_settings_api_root', '_settings_root', '_settings_service', '_sync_from_future', '_system_coupling', '_tui', '_tui_service', '_version', '_workflow', '_workflow_se', 'build_from_fluent_connection', 'connection_properties', 'create_from_server_info_file', 'datamodel_events', 
'datamodel_service_se', 'datamodel_service_tui', 'download', 'error_state', 'events_manager', 'events_service', 'execute_tui', 'exit', 'field_data', 'field_data_streaming', 'field_info', 'fluent_connection', 'force_exit', 'force_exit_container', 'get_fluent_version', 'health_check_service', 'id', 'journal', 'monitors_manager', 'preferences', 'read_case', 'read_case_lightweight', 'reduction', 'rp_vars', 'scheme_eval', 'settings_service', 'start_journal', 'stop_journal', 'svar_data', 'svar_info', 'svar_service', 'system_coupling', 'transcript', 'tui', 'upload', 'version', 'workflow', 'write_case']

It is missing file, mesh, setup, solution, etc., regardless of existing codegen files (same behavior in v222 and v242). If I do any settings API call (which requests the static info), then after that point the dir() method includes the previously missing attributes/objects, as expected.

For comparison, in the main branch without these changes doing the same thing I get:

 >>> session = pf.launch_fluent()
>>> dir(session)
['__call__', '__class__', '__delattr__', '__dict__', '__dir__', '__doc__', '__enter__', '__eq__', '__exit__', '__format__', '__ge__', '__getattribute__', '__getstate__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__le__', '__lt__', '__module__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', '__weakref__', '_batch_ops_service', '_build_from_fluent_connection', '_field_data_service', '_lck', '_monitors_service', '_pim_methods', '_preferences', '_reduction_service', '_remote_file_handler', '_root', '_se_service', '_settings_root', '_settings_service', '_sync_from_future', '_system_coupling', '_tui', '_tui_service', '_version', '_workflow', '_workflow_se', 'build_from_fluent_connection', 'connection_properties', 'create_from_server_info_file', 'current_parametric_study', 'datamodel_events', 'datamodel_service_se', 'datamodel_service_tui', 'error_state', 'events_manager', 'events_service', 'execute_tui', 'exit', 'field_data', 'field_data_streaming', 'field_info', 'file', 'fluent_connection', 'force_exit', 'force_exit_container', 'get_fluent_version', 'health_check_service', 'id', 'journal', 'mesh', 'monitors_manager', 'parallel', 'parameters', 'parametric_studies', 'preferences', 'read_case', 'read_case_lightweight', 'reduction', 'report', 'results', 'rp_vars', 'scheme_eval', 'server', 'settings_service', 'setup', 'solution', 'start_journal', 'stop_journal', 'svar_data', 'svar_info', 'svar_service', 'system_coupling', 'transcript', 'tui', 'version', 'workflow', 'write_case']

Which as you can see has all of the missing attributes from the start.

Is this a known limitation of this approach?

Should we improve the __dir__() method so that it gets the attributes and properly populates the dir? As @mkundu1 suggested, I believe this could be done using the codegen files without needing to get the static info from the server.

@prmukherj
Copy link
Collaborator Author

prmukherj commented Dec 14, 2023

Good point @raph-luc. I have updated the dir() with static info call. We are avoiding the use of codegen files for solver. Instead of that we are using static info call once to populate the list. Please point out if something else can be done. Thank you

@prmukherj prmukherj merged commit 2d72241 into main Jan 4, 2024
20 checks passed
@prmukherj prmukherj deleted the feat/expose_settings_root_implicitly branch January 4, 2024 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bogus settings report object?
3 participants