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

Isolate serialization of python objects in G3PipelineInfo interface #147

Merged
merged 28 commits into from
Mar 3, 2024

Conversation

arahlin
Copy link
Member

@arahlin arahlin commented Mar 1, 2024

This PR adds logic throughout the G3PipelineInfo and G3ModuleConfig class definitions to enable them to serialize appropriately in a pure-C++ program. Notably, any serialization of python objects is handled only on interface with the python interpreter, and otherwise stored in C++ memory with the string representation.

arahlin and others added 19 commits February 27, 2024 23:57
This PR creates a new class that simplifies initialization of python threads, as
well as acquiriing / releasing the Python global interpreter lock in various
contexts.

Use cases include:

1. Ensuring that Py_Initialize() is properly called at the beginning of a
   program that is expected to interact with the python interpreter, and also
   that Py_Finalize() is called when the program is finished.
2. Ensuring that the current thread state is saved and the GIL released as
   necessary, e.g. for IO operations, and then the thread state is restored on
   completion.
3. Ensuring that the GIL is acquired for one-off interaction with the python
   interpreter, and released when complete.

A G3PythonContext object is used throughout the library code for cases 2 and 3.
If the python interpreter has not been initialized (i.e. the compiled program is
expected to be purely in C++), then these context objects are essentially no-op.
If the python interpreter is initialized (e.g. inside a python program or
command-line interface), then these context objects will handle the GIL
appropriately.

See the examples/cppexample.cxx C++ program for a simple implementation of the
above behavior.

This PR also adds logic throughout the G3PipelineInfo and G3ModuleConfig
class definitions to enable them to serialize appropriately in a pure-C++
program.
These are python objects, and if we allow them to be deleted otherwise,
bad things happen. This fixes at least most of the concurrency problems
I have with reading files that have G3PipelineInfo in them?
Use a G3MapFrameObject storage structure for the module arguments, rather than a
map of python objects.  Since the serialization process requires a call to
repr() for non-G3FrameObjects anyway, do this step in the python shim that
creates the config in the first place.

Also ensure that simple scalar values are serialized as frame objects.

Adds a new ``spt3g.core.to_g3frameobject`` function for converting
python objects to G3FrameObjects.
@arahlin arahlin requested a review from nwhitehorn March 1, 2024 02:19
@arahlin arahlin self-assigned this Mar 1, 2024
@cozzyd
Copy link
Contributor

cozzyd commented Mar 1, 2024

This looks good to me!

@nwhitehorn
Copy link
Member

Just so I understand fully from reading the code: every object gets repr() called on it and the repr() of everything is now stored. For frame objects, the frame object is also serialized. On deserialization, any frame objects are instantiated, but things where just the repr() is stored are only evaluated lazily. Is that correct?

@arahlin
Copy link
Member Author

arahlin commented Mar 2, 2024

Yeah that's right. I'm going to rearrange a bit so that the repr for frame objects in v1 files doesn't need to be grabbed on load, so that we don't need to grab the GIL in C++ only code ever.

@nwhitehorn
Copy link
Member

That sounds great. I'll hold off formally approving until those commits are in, but this is a really nice solution to the problem.

@arahlin
Copy link
Member Author

arahlin commented Mar 3, 2024

Should be all set now. Thanks!

Copy link
Member

@nwhitehorn nwhitehorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@arahlin arahlin merged commit f5cd59f into master Mar 3, 2024
1 check passed
@arahlin arahlin deleted the modconfig_refactor branch March 3, 2024 15:23
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