BradGreig / Hybrid21CM

1 stars 3 forks source link

warnings from C code #15

Closed steven-murray closed 5 years ago

steven-murray commented 5 years ago

At the moment, if there's a warning from the C code it immediately gets printed to screen. It would be nice to be able to use, when appropriate, the Python warning system, which catches them and only prints the same warning once per session. Otherwise in an MCMC situation you can end up with a huge amount of redundant warnings.

Not sure exactly how to do this. It may be that for all the warnings you have, we can literally just move the check up to the python level and emit the warning from there. We could compile the warnings into the C code based on a DEFINE statement which can be controlled at compile time (by default turned off when compiling from setup.py).

Otherwise, we could look at trying to capture all C output from python, and turning it into a formal warning or otherwise depending on the contents.

BradGreig commented 5 years ago

There shouldn't be too many warning's on the C side. Especially related to the parameter choices/setup. Most of the actual warnings are cell specific, which makes it impossible for Python to capture.

I guess the best solution to this would be for C to catch all warnings etc. in a string, which can be passed back to Python and handled appropriately from there.

steven-murray commented 5 years ago

OK, yeah obviously for all warnings that are getting emitted deep in the code, we can't do that from Python, so it'll have to stay in C. The one I was thinking of primarily at the moment is this:

perturb_field.c: WARNING: Resolution is likely too low for accurate evolved density fields It Is recommended that you either increase the resolution (DIM/Box_LEN) or set the EVOLVE_DENSITY_LINEARLY flag to 1

I feel like this is probably something we could predict from Python, though I'm not sure of the details.

I'm pretty sure there would be a way to automatically capture the stdout that C emits from the calling Python function, and then emit it in whatever way we choose when the call returns. However, for warnings that are cell-specific and could lead to segfaults or something, it's probably better to just let them get written out so that the user can see them. Maybe we should (eventually) have a quick look through at all the stuff you're writing from C, and determine whether it might be better to vet it before writing or not.

BradGreig commented 5 years ago

Ah, ok. I can have a look through the code to see which are/aren't capable of being moved to Python. I was clearly thinking of a couple of other warning's. I've actually removed a lot already, many more exist in 21cmFAST.

steven-murray commented 5 years ago

Yeah warnings are definitely good... but when they keep coming through an MCMC it can get a bit tedious!

On Tue, Dec 4, 2018 at 7:51 PM BradGreig notifications@github.com wrote:

Ah, ok. I can have a look through the code to see which are/aren't capable of being moved to Python. I was clearly thinking of a couple of other warning's. I've actually removed a lot already, many more exist in 21cmFAST.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/BradGreig/Hybrid21CM/issues/15#issuecomment-444339563, or mute the thread https://github.com/notifications/unsubscribe-auth/ABNo3m5kXYpN-xtsrv3T8auieGvLJliMks5u1zShgaJpZM4ZBtn0 .

-- Steven Murray, Research Associate at School of Earth and Space Exploration, Arizona State University.

BradGreig commented 5 years ago

Can you construct the framework for this, and I'll go through and add all the warnings that can be removed from C.

steven-murray commented 5 years ago

I'll give it a go, but have some other bugs/things to fix first. Trying to get all the v1 milestones done (basically trying to make the split between stuff that is required before publication, and stuff that can be done after)

steven-murray commented 5 years ago

Moved to referenced issue.