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

Implement logger for AbacusHOD #134

Merged
merged 5 commits into from
May 9, 2024
Merged

Implement logger for AbacusHOD #134

merged 5 commits into from
May 9, 2024

Conversation

epaillas
Copy link
Contributor

@epaillas epaillas commented May 8, 2024

Hi everyone,

Since AbacusHOD is now regularly run in conjuction with other (DESI) packages, it would be nice to have a logger that would allow the user to define the level of the standard output messages, in case they want to completely mute the prints, only print warnings, etc.

This pull request implements the logger for a few of the main HOD sampling routines. It can be initialized as

from abacusnbody.hod import setup_logging

setup_logging()

It follows the same convention as other cosmodesi modules, so we only need to initialize it once from any package, and then the output will be shared:

[000171.96]  05-08 13:53  AbacusHOD                    INFO     HOD generated in elapsed time 0.36 s.
[000172.06]  05-08 13:53  TwoPointCorrelationFunction  INFO     Using estimator <class 'pycorr.twopoint_estimator.NaturalTwoPointEstimator'>.
[000172.06]  05-08 13:53  TwoPointCorrelationFunction  INFO     Running auto-correlation.
[000172.06]  05-08 13:53  TwoPointCorrelationFunction  INFO     Computing two-point counts D1D2.
[000173.20]  05-08 13:53  TwoPointCorrelationFunction  INFO     Analytically computing two-point counts R1R2.
[000173.20]  05-08 13:53  TwoPointCorrelationFunction  INFO     Correlation function computed in elapsed time 1.14 s.

Let me know if this works.

Cheers,
Enrique

@lgarrison
Copy link
Member

Thanks for the contribution! This looks like a great improvement.

It looks like the CI config was a little messed up so the tests didn't run. I think I've fixed it; can you rebase on main?

Also, since the logging utils were taken from desilike, I think the BSD license needs to be reproduced alongside the source. Since it's just utils.py, I would consider copying the license from https://github.com/cosmodesi/desilike/blob/hmc/LICENSE into the top of that file.

@epaillas
Copy link
Contributor Author

epaillas commented May 9, 2024

I rebased to the main branch, but I think some of the checks are still failing. Is there anything else you'd like me to try?

@lgarrison
Copy link
Member

Yep, ruff is flagging the following:

abacusnbody/hod/__init__.py:1:20: F401 `.utils.setup_logging` imported but unused; consider removing, adding to `__all__`, or using a redundant alias

(You can see this by clicking "Details" next to the failing pre-commit check)

I would adopt the suggested fix of adding __all__ = ['setup_logging'] to that file.

@lgarrison
Copy link
Member

Looks good, thanks!

@lgarrison lgarrison merged commit ce7e184 into abacusorg:main May 9, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants