MolSSI / QCFractal

A distributed compute and database platform for quantum chemistry.
https://molssi.github.io/QCFractal/
BSD 3-Clause "New" or "Revised" License
144 stars 47 forks source link

[next branch] no logs when `verbose=False` #728

Closed hadim closed 1 year ago

hadim commented 1 year ago

At https://github.com/MolSSI/QCFractal/blob/next/qcfractalcompute/qcfractalcompute/qcfractal_manager_cli.py#L712, the INFO logs are shown if I replace:

    logger_map = {AdapterEnum.pool: "", AdapterEnum.dask: "dask_jobqueue.core", AdapterEnum.parsl: "parsl"}
    if settings.common.verbose:
        adapter_logger = logging.getLogger(logger_map[settings.common.adapter])
        adapter_logger.setLevel("DEBUG")
        logger.setLevel("DEBUG")

by

    logger_map = {AdapterEnum.pool: "", AdapterEnum.dask: "dask_jobqueue.core", AdapterEnum.parsl: "parsl"}
    adapter_logger = logging.getLogger(logger_map[settings.common.adapter])
    if settings.common.verbose:
        adapter_logger.setLevel("DEBUG")
        logger.setLevel("DEBUG")
    else:
        adapter_logger.setLevel("INFO")
        logger.setLevel("INFO")
hadim commented 1 year ago

Again here it might not be relevant anymore given that qcfractal-compute-manager is prefered.

hadim commented 1 year ago

@bennybp I still cannot see the logs when using INFO level with qcfractal-compute-manager.

It works if I specify the name of the logger in compute_manager_cli.py:

    # Adjust the logging level to what was in the config
    logging.getLogger().setLevel(manager_config.loglevel)

    if manager_config.logfile:
        print("*" * 10)
        print(f"Logging to file {manager_config.logfile}, logging level {manager_config.loglevel}")
        print("*" * 10)
        log_handler = logging.FileHandler(manager_config.logfile)
        log_handler.setFormatter(formatter)
        logging.getLogger().handlers = [log_handler]

to

    # Adjust the logging level to what was in the config
    logging.getLogger("ComputeManager").setLevel(manager_config.loglevel)

    if manager_config.logfile:
        print("*" * 10)
        print(f"Logging to file {manager_config.logfile}, logging level {manager_config.loglevel}")
        print("*" * 10)
        log_handler = logging.FileHandler(manager_config.logfile)
        log_handler.setFormatter(formatter)
        logging.getLogger("ComputeManager").handlers = [log_handler]
bennybp commented 1 year ago

I will have to test, but I think I need to .clear() the handlers, then add the new handlers (rather than what I did). I think that is the more proper way to do that (since some logging code may still be pointing at the old handler list, I need to modify in-place)

bennybp commented 1 year ago

I'm having trouble reproducing with the new manager. It always seems to print whether or not I specify --verbose. Do you have an example config/env vars/command line?

hadim commented 1 year ago

I am not using --verbose simply qcfractal-compute-manager --config config.yaml

# The base folder to use as the default for some options (logs, etc).
base_folder: /tmp/qcf_compute

# Name of this scheduler to present to the Fractal Server. Descriptive names help
# the server identify the manager resource and assists with debugging.
cluster: manager_demo_1

loglevel: INFO
logfile: null

# Time between heartbeats/update checks between this Manager and the Fractal Server.
update_frequency: 30

# Settings to connect to the Fractal Server.
server:
  fractal_uri: http://127.0.0.1:7777
  username: compute_user
  password: compute_password
  verify: false

# How and where to detect the QM softwares.
environments:
  use_manager_environment: true
  conda: []
  apptainer: []

executors:
  local:
    type: local

    # Common to all executors.
    queue_tags: ["demo"]
    worker_init: []
    scratch_directory: null
    bind_address: null
    cores_per_worker: 16
    memory_per_worker: 16 # GB
    extra_executor_options: {}

    # Specific options for the local executor.
    max_workers: 1
bennybp commented 1 year ago

I think what might be happening is that the base_folder set in the config isn't being respected, and just being set to the directory the config is in. Is there a qcfractal_compute.log file in that directory?

hadim commented 1 year ago

Oh indeed, it's because the file handler is removed instead of being added: logging.getLogger("ComputeManager").handlers = [log_handler]

bennybp commented 1 year ago

oh disregard what I said. I think it's simpler than that. If you specify null in the config, then https://github.com/MolSSI/QCFractal/blob/396dcd967fda5f979bc95e4ddf1751cdb147e7ae/qcfractalcompute/qcfractalcompute/config.py#L163 still gets run. A simple check there should be sufficient.

    @validator("logfile")
    def _check_logfile(cls, v, values):
        if v is None:
            return None
        return _make_abs_path(v, values["base_folder"], "qcfractal_compute.log"
hadim commented 1 year ago

Do you think we should actually propose the option to have the base_folder set to null? While useful in some cases, it's not in some other cases as well. For example, in a cloud serverless container-based setting.

bennybp commented 1 year ago

I think that would be appropriate. It's really only used as the default path for some things, and usually just set to be the same directory as where the config file is. If there is no need for that, then it should be allowed to be null or somehow just not set

bennybp commented 1 year ago

Original logging issue should be fixed in https://github.com/MolSSI/QCFractal/commit/25f98de13ddd7b16b443876f43ad2d6ae7993aa0

hadim commented 1 year ago

I confirm the fix. Thanks. I will open a new ticket for base_folder