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

e3sm_to_cmip --help creates a logs directory #220

Closed
TonyB9000 opened this issue Oct 5, 2023 · 6 comments · May be fixed by #281
Closed

e3sm_to_cmip --help creates a logs directory #220

TonyB9000 opened this issue Oct 5, 2023 · 6 comments · May be fixed by #281

Comments

@TonyB9000
Copy link
Contributor

TonyB9000 commented Oct 5, 2023

Simply issuing "e3sm_to_cmip --help" creates a "logs" directory in your current working directory, and an empty logfile.
(version 1.10.0rc1). Same for "e3sm_to_cmip --version".

@TonyB9000
Copy link
Contributor Author

@tomvothecoder @chengzhuzhang (Just wondering if you have any insights here, before I begin culling/refactoring code.)

In main.py, soon after the import:

from e3sm_to_cmip._logger import _setup_logger, _setup_root_logger

a global variable "log_filename" is defined:

# Setup the root logger and this module's logger.
log_filename = _setup_root_logger()
logger = _setup_logger(__name__, propagate=True)

The _setup_root_logger() creates a local "logs/" directory, LONG BEFORE we get a chance to parse any command line options.

The ONLY place this global variable is addressed is in the instantiation of class E3SMtoCMIP, where it simply serves to announce the logfile path (ironically, into the logfile):

logger.info(f" * Writing log output file to: {log_filename}")

I will remedy this situation by moving the call to _setup_root_logger() to a point within the class, AFTER arg-parsing is completed - and we can check if "--help" or "--version" has been invoked.

For Additional Edification:

There is a setup_cmor() defined in util.py: It creates a local "logs" directory.

There is a setup_cmor() defined in mpas.py. It creates a local "cmor_logs" directory.

The ONLY codes that calls either of these functions (specifically, the one in util.py) are these 3 cmor_handlers for mpas vars:

cmor_handlers/mpas_vars/areacello.py:from e3sm_to_cmip.util import print_message, setup_cmor
cmor_handlers/mpas_vars/areacello.py:    setup_cmor(var_name=VAR_NAME, table_path=tables, table_name=TABLE,
cmor_handlers/mpas_vars/thkcello.py:from e3sm_to_cmip.util import print_message, setup_cmor
cmor_handlers/mpas_vars/thkcello.py:    setup_cmor(var_name=VAR_NAME, table_path=tables, table_name=TABLE,
cmor_handlers/mpas_vars/volcello.py:from e3sm_to_cmip.util import print_message, setup_cmor
cmor_handlers/mpas_vars/volcello.py:    setup_cmor(var_name=VAR_NAME, table_path=tables, table_name=TABLE,

Add to this, there is a _setup_cmor_module() defined in cmor_handlers/handler.py, called within "cmorize()'. It will also create a local "cmor_logs" directory unless "log_dir" is passed in as a parameter. But the only call to "cmorize" found is in the

cmor_handlers/handler.py: return {**self.__dict__, "method": self.cmorize}

Specifically via:

def to_dict(self) -> Dict[str, Any]:
    # TODO: Remove this method e3sm_to_cmip functions parse VarHandler
    # objects instead of its dictionary representation.
    return {**self.__dict__, "method": self.cmorize}

which appears to invoke only the default parameter values. The "help" declares:

--logdir LOGDIR Where to put the logging output from CMOR.

processed by

        optional.add_argument(
            "--logdir",
            type=str,
            default="./cmor_logs",
            help="Where to put the logging output from CMOR.",
        )

and assigned via

__main__.py:    logdir: Optional[str]
__main__.py:        self.cmor_log_dir: Optional[str] = parsed_args.logdir

In testing, setting "--logdir=<user_defined_path>" had no effect upon the local creation of 'logs/".

As if this is not enough, the "_logger.py" module ( def _setup_root_logger() ) ALSO creates a local "logs/" directory.

This issue (#220), as well as (#229 "Logger is printing duplicate messages") should be fixed in due course.

@chengzhuzhang
Copy link
Collaborator

@TonyB9000 it would be nice to fixing logger. It is minor but has been an annoyance. It looks like you already have a good plan to fix, if in this case go for it. We do want to place priority on the Omon/SImon issue though.

@TonyB9000
Copy link
Contributor Author

@chengzhuzhang I agree, and I will attempt to use Tom's "python-test" method of running the e2c test, so that in-depth debugging can be employed. That, however, will be a very slow process for me - learning curve, etc.

@TonyB9000
Copy link
Contributor Author

Moving the _setup_root_logger() until AFTER arg_parsing is called suffices to solve the unwanted "logs/" directory creation upon "--help" or "--version", as the process exits immediately after arg parsing in such cases.

The issue of log-duplication (and triplication), #229, will be addressed separately.

@TonyB9000
Copy link
Contributor Author

In Addition: Moving _setup_root_logger() until AFTER arg_parsing allows that the argument "--logdir" can now be passed into the logger setup, overriding the default "logs/" for routine processing. This was never captured as a separate issue.

I intend to test with a version eliding the "console_handler" codes, just to see if this is driving the message duplication. Routinely, I issue <command> . . . > output_log 2>&1 anyway, so I never make use of console output at runtime.

@tomvothecoder
Copy link
Collaborator

Duplicate of #274

@tomvothecoder tomvothecoder marked this as a duplicate of #274 Oct 16, 2024
@github-project-automation github-project-automation bot moved this from In progress to Done in E3SM to CMIP Development Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
3 participants