Skip to content

Logging rework#1651

Merged
raph-luc merged 6 commits intomainfrom
feat/logging_rework
Jun 7, 2023
Merged

Logging rework#1651
raph-luc merged 6 commits intomainfrom
feat/logging_rework

Conversation

@raph-luc
Copy link
Copy Markdown
Member

@raph-luc raph-luc commented May 29, 2023

Closes #1259, #1625, #1532 and I believe also #9 (see discussion below)

Changes:

  • Logging to files is now by default disabled (still logs warning and above to console stdout) and users need to manually enable it or set an environment variable so that it is enabled automatically on startup (more below)
  • PyFluent loggers now have the pyfluent prefix to avoid conflict with other loggers
  • All logging output now goes to a single file, example:
    Logging file example image
  • Added support for PYFLUENT_LOGGING environment variable, which we can use so that we don't have to change logging levels manually and so that logging is automatically set to DEBUG (or any other level you want) for development/debugging
  • Added convenience functions list_loggers() and set_global_level() (see documentation in the item below)
  • Add documentation:
    User guide index
    image
    API reference index (this isn't directly related to Fluent's API, but couldn't think of a better place in the current docs, please let me know of any suggestions)
    image

Recommendations:

  • I think using root logging calls such as logging.info() or logging.warning() is something we should avoid and could conflict/cause issues with other packages or something that users have set up themselves, using the custom defined loggers is better in every way that I can think of, so logger=getLogger('pyfluent...'); logger.info() etc
  • Use print() when the objective is to output non-error information to the Python console directly for the user to see or when it is an important non-error message and just logging to files is insufficient (we can set up additional logging for different combinations as well if necessary)

In regards to #9, the only scenario where no useful information or silent failing was being returned to the user that I could think of, and based on my experience as this was an issue for me too, was when running Fluent on containers, where it was failing completely silently with no logs or console output at all to help. My suggested fix (relevant lines below) is to set the docker image to run on the specified work directory that is also mounted on the container host system. In this way, the user, which has access to the host system but not necessarily the guest docker container files, now has access to the Fluent automatic transcript output too, which is typically what is needed to better debug launch issues. I have also changed all the docker run shorthand args for their full names (check the other lines around the ones linked below), as I believe this makes them much clearer and easier to work with. Please let me know if I missed any scenarios where logging and workdir/transcript location changes aren't sufficient.

"--workdir",
f"{mounted_to}",

While this is fresh on my mind, please let me know if you have any questions, can think of any suggestions or ways in which logging would be more useful to you for development or any other purposes.

Copy link
Copy Markdown
Contributor

@mkundu1 mkundu1 left a comment

Choose a reason for hiding this comment

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

I think logging into a single file by default is a better idea as the order of the operations can easily be understood by looking into a single file.

@raph-luc
Copy link
Copy Markdown
Member Author

The changes exposed what I believe is a flaw in ansys/actions ansys/actions#269, causing all of the tests that use build-wheelhouse to fail

Fixed some of the tests related to the broken github actions from PyFluent's side. For the ansys/actions side, the best way to fix this in the long term is going to require changes on their repo which I currently don't have permissions to create branches/PRs, so I just created the issue tracker for now

@raph-luc raph-luc added bug Issue, problem or error in PyFluent documentation Documentation related (improving, adding, etc) enhancement Improve any current implemented feature refactoring labels May 29, 2023
@raph-luc
Copy link
Copy Markdown
Member Author

raph-luc commented Jun 1, 2023

Left an useful debug args for docker run commented here, may be useful in the future for others as well, let me know if you prefer that I remove it:

# "--shm-size", # controls the amount of memory allocated, useful for debugging
# "512MiB",

@raph-luc raph-luc marked this pull request as ready for review June 1, 2023 20:28
@raph-luc raph-luc requested review from acarvalh-work and mkundu1 June 1, 2023 20:33
@raph-luc raph-luc merged commit 3c32398 into main Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue, problem or error in PyFluent documentation Documentation related (improving, adding, etc) enhancement Improve any current implemented feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Set global log level in 0.13 Doc for new logging infrastructure Assess logging framework Logging Fluent launch errors

5 participants