-
Notifications
You must be signed in to change notification settings - Fork 4
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
Inconsistent Behaviour in use of custom_log
#629
Comments
Thanks, Tim. Think I've found the cause of this problem. As you demonstrate, there are two ways to configure logging levels for the modules: either passing the Unfortunately, the [internal] logic is not consistent. When passing the I'll fix this to make it consistent. I think it's safe to apply all the levels in |
@tamuri, @tbhallett . Is this issue resolved? I can see it was closed as completed, then re-opened, moved to done and then moved back to to do. Are there any issues still standing? |
Thanks for picking this up, Emmanuel. I think it still needs works but safest would be if you could check it yourself and confirm one way or the other!
Sent from Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: Emmanuel Mnjowe ***@***.***>
Sent: Tuesday, April 16, 2024 10:20:40 AM
To: UCL/TLOmodel ***@***.***>
Cc: Hallett, Timothy B ***@***.***>; Mention ***@***.***>
Subject: Re: [UCL/TLOmodel] Inconsistent Behaviour in use of `custom_log` (Issue #629)
This email from ***@***.*** originates from outside Imperial. Do not click on links and attachments unless you recognise the sender. If you trust the sender, add them to your safe senders list<https://spam.ic.ac.uk/SpamConsole/Senders.aspx> to disable email stamping for this address.
@tamuri<https://github.com/tamuri>, @tbhallett<https://github.com/tbhallett> . Is this issue resolved? I can see it was closed as completed, then re-opened, moved to done and then moved back to to do. Are there any issues still standing?
—
Reply to this email directly, view it on GitHub<#629 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AJRDOFGZEYBAQWTUXDPPVU3Y5TUORAVCNFSM5Y3X7Q32U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMBVHA3DGMRQHAZA>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Hi @tbhallett . I think this issue was resolved. Have run the two scenarios above and they are all passing. Have also run another issue #840 linked to this issue and its behaving just as expected. I was just thinking though if it can be good to make private |
Agreed -- please propose that change in a PR for @tamuri 's review |
@mnjowe --- could I also suggest that that test ( |
That's a good suggestion @tbhallett. I agree |
Thanks @mnjowe. Please raise a PR for this when you get the chance and we'll get this ticked off! 👍 |
Hi @tbhallett. I can see this issue in linked to PR #1157. Should I continue with that PR or I should still open a new PR? PR #1157 is still open |
Either would be fine. As this PR has the key stuff already, it would be pretty easy to use it, so maybe do that! |
When using the argument
custom_log
when initialising a simulation, behaviour is not as expected when there is more than logger defined within a module. The behaviour is as expected when using the methodsim.configure_logging()
after the models have been registered.As a consequence, all runs that ask for
tlo.methods.demography
(which is most) also gotlo.methods.demography.detail
(which they don't want and is very heavy).The issue is demonstrated in the tests added in #628
The text was updated successfully, but these errors were encountered: