search

clessig / atmorep

AtmoRep model code
MIT License
44 stars 11 forks source link

closes issue #19 #23

Open grassesi opened 3 months ago

grassesi commented 3 months ago

import logger as it is setup in utils.logger module and uses logger.info() where ever print is used. Commented out prints were converted into logger.debug(). Also imports unused imports were removed throughout. For core.train.py, core.train_multi.py and core.evaluator.py the convention to import only modules was enforced for imports from utils.utils. closes #19 .

clessig commented 3 months ago

Hi @grassesi , many thanks for getting started at this. I just tried to run the changes and I get a long list of errors like this:

--- Logging error --- Traceback (most recent call last): File "/usr/local/apps/python3/3.10.10-01/lib/python3.10/logging/init.py", line 1100, in emit msg = self.format(record) File "/usr/local/apps/python3/3.10.10-01/lib/python3.10/logging/init.py", line 943, in format return fmt.format(record) File "/lus/h2resw01/hpcperm/nacl/research/atmorep/develop/atmorep/atmorep/utils/logger.py", line 14, in format return super().format(record) File "/usr/local/apps/python3/3.10.10-01/lib/python3.10/logging/init.py", line 678, in format record.message = record.getMessage() File "/usr/local/apps/python3/3.10.10-01/lib/python3.10/logging/init.py", line 368, in getMessage msg = msg % self.args TypeError: not all arguments converted during string formatting Call stack: File "/lus/h2resw01/hpcperm/nacl/research/atmorep/develop/atmorep/atmorep/core/train.py", line 237, in train() File "/lus/h2resw01/hpcperm/nacl/research/atmorep/develop/atmorep/atmorep/core/train.py", line 227, in train cf.print() File "/lus/h2resw01/hpcperm/nacl/research/atmorep/develop/atmorep/atmorep/utils/utils.py", line 66, in print logger.info("{} : {}", key, value)

Is there a specific python version of package that is required?

grassesi commented 3 months ago

Hi @clessig, it should be fixed now. By default logging only supports old %s style formatting, but i used modern "{}".format style. To fix this I added support in utils.logger.py by using logging.setLogRecordFactory to set a custom LogRecord that supports both formatting styles. I added unit tests to test this behavior.

grassesi commented 3 months ago

btw currently all logging output is directed towards sys.stderr (default behavior of StreamHandler), so it probably wont show up in the slurm output file. Is this inteded, or should we discuss what output we actually need in which file ?