VegeWaterDynamics / motrainer

Distributed Measurement Operator Trainer for Data Assimilation Applications
https://vegewaterdynamics.github.io/motrainer/
Apache License 2.0
2 stars 0 forks source link

[JOSS] Recommend placing `os.environ` assignments inside conditional #126

Closed WarmCyan closed 3 months ago

WarmCyan commented 4 months ago

In model.py and util.py you're creating an environment variable to set a tensorflow log level:

https://github.com/VegeWaterDynamics/motrainer/blob/f74acd8c44cc0be7539095c8a6ab4c0c1560f6e4/motrainer/model.py#L12

https://github.com/VegeWaterDynamics/motrainer/blob/f74acd8c44cc0be7539095c8a6ab4c0c1560f6e4/motrainer/util.py#L11

I think it's generally good practice for libraries to avoid potentially overriding user-defined config. I would recommend putting both of these inside a conditional so that it only sets the environment variable if it doesn't already exist. Something like:

if "TF_CPP_MIN_LOG_LEVEL" not in os.environ:
    os.environ["TF_CPP_MIN_LOG_LEVEL"] = "2"

This would let the user still set the log level to include debug messages if they were having issues with something (I've run into this problem before elsewhere!)