Skip to content

Flush stdout if a custom model printer has been registered.#610

Open
rkaminsk wants to merge 1 commit intowip-20from
fix/flush-print-model
Open

Flush stdout if a custom model printer has been registered.#610
rkaminsk wants to merge 1 commit intowip-20from
fix/flush-print-model

Conversation

@rkaminsk
Copy link
Member

Output might get tangled if a model printer is registered in Python because python manages its own output buffer. I pushed a preliminary fix. @BenKaufmann what do you think about this? Should we flush the output in general or should we just do it for the Python API? For the C/C++ APIs there should be no issues because here we do not have separate buffers.

@rkaminsk rkaminsk requested a review from BenKaufmann March 13, 2026 10:42
@rkaminsk rkaminsk force-pushed the fix/flush-print-model branch 2 times, most recently from ab5ee3a to 3afccea Compare March 13, 2026 12:13
@rkaminsk
Copy link
Member Author

rkaminsk commented Mar 13, 2026

With the last force push, this is handle on the Python side only:

// ensure OS buffer is flushed
std::cout.flush()
app.print_model(Model{model}, [printer, printer_data]() { handle_error(printer(printer_data)); });
// ensure Python buffer is flushed
py::module_::import("sys").attr("stdout").attr("flush")();

The two flushes are important because it seems like python directly writes large buffers without flushing previously written smaller buffers leading to tangled up output.

@rkaminsk rkaminsk force-pushed the fix/flush-print-model branch from 3afccea to 1d0d9cb Compare March 13, 2026 13:39
@jorgefandinno
Copy link

If I remember correctly c functions fwrite and write also may use different buffers in some systems. So, this issue may have happened in the c-api for some systems.

@rkaminsk
Copy link
Member Author

If I remember correctly c functions fwrite and write also may use different buffers in some systems. So, this issue may have happened in the c-api for some systems.

It may not be required by the standard but printf and fprintf to stdout not using the same buffer would be strange. In any case, users should stick to printf when using the C API. So I don't see a problem. And for other language bindings this has to be addressed on a case by case basis.

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.

3 participants