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

make FEMap a gufe tokenizable #108

Open
richardjgowers opened this issue Nov 6, 2023 · 4 comments
Open

make FEMap a gufe tokenizable #108

richardjgowers opened this issue Nov 6, 2023 · 4 comments
Assignees

Comments

@richardjgowers
Copy link
Contributor

To allow serialisation between sessions

@richardjgowers
Copy link
Contributor Author

I started playing with this, and one big change is that we'd have to make the FEMap object immutable. Is this an ok choice for this? @dwhswenson opinions?

The alternative would be we'd just make the to_dict/from_dict not via gufe.

@ijpulidos
Copy link
Collaborator

Do we need to make it gufe tokenizable or just serializable? I think we would like to make the object easy to extend for the "live networks" plan that we have for alchemiscale and getting results from it. I guess this could be done with immutable objects (creating a new one every time), but may not be necessary.

@dwhswenson
Copy link
Member

I'm not sure enough about the use cases here. GufeTokenizable adds a few potential conveniences (versioning of serialization, ease of use when saving to gufe-based storage, a few useful points on provenance, etc). But I'm not familiar enough with the needs of cinnabar users to say whether that is worth dropping mutability.

I'll argue strongly for immutable objects if they're likely to be stored as part of a larger database. If the serialization is always directly to a single file for that object, I feel like mutability causes fewer problems (it is understood that you're overwriting the old file). However, when you save into a larger database, mutability requires that the database manage updates, and you lose provenance tracking. That mainly matters if you're using objects that are then themselves used as inputs elsewhere; I'm not sure how much that would happen with an FEMap.

@richardjgowers
Copy link
Contributor Author

Immutable is a bit misleading, it just means that the pattern for updating a network behaves differently

# mutable
m = FEMap()

for line in myfile:
    m.add_absolute_measurement(line)

# immutable
m = FEMap()
for line in myfile:
    m = m.add_absolute_measurement(line)

Maybe a simple to/from dict is enough for FEMap though

@richardjgowers richardjgowers moved this from Planned to Sprint - In Progress in Perses-OpenFE roadmap Nov 20, 2023
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

No branches or pull requests

3 participants