Skip to content

docs: Context and Storage#712

Open
ethanholz wants to merge 18 commits intoOpenFreeEnergy:feat/warehousefrom
ethanholz:docs/context
Open

docs: Context and Storage#712
ethanholz wants to merge 18 commits intoOpenFreeEnergy:feat/warehousefrom
ethanholz:docs/context

Conversation

@ethanholz
Copy link
Contributor

@ethanholz ethanholz commented Jan 21, 2026

This PR closes #695, providing documentation on ExternalStorage and Context.

Checklist

  • Added a news entry

Developers certificate of origin

Signed-off-by: Ethan Holz <ethan.holz@omsf.io>
@codecov
Copy link

codecov bot commented Jan 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (feat/warehouse@c932c8b). Learn more about missing BASE report.

Additional details and impacted files
@@                Coverage Diff                @@
##             feat/warehouse     #712   +/-   ##
=================================================
  Coverage                  ?   98.33%           
=================================================
  Files                     ?       42           
  Lines                     ?     2526           
  Branches                  ?        0           
=================================================
  Hits                      ?     2484           
  Misses                    ?       42           
  Partials                  ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ethanholz ethanholz changed the title [WIP] docs: Context and Storage docs: Context and Storage Jan 21, 2026
@ethanholz ethanholz marked this pull request as ready for review January 21, 2026 23:55
@ethanholz
Copy link
Contributor Author

Closes #695 . @atravitz ready for review whenever you get to it.

@atravitz atravitz self-requested a review January 22, 2026 21:04
@ethanholz
Copy link
Contributor Author

@atravitz pinging on the feedback from #718, we already demonstrate the workflow for loading data from the a result and using it in a protocol so I think this is already covered.

@ethanholz
Copy link
Contributor Author

One thing that is clearly missing here is an update to the Protocol How-To, is this work that someone on the OpenFE side would like to tackle? I am more than happy to assist but I think it might be a good exercise in knowledge transfer.

@ethanholz ethanholz requested a review from atravitz January 28, 2026 20:42
@atravitz
Copy link
Contributor

One thing that is clearly missing here is an update to the Protocol How-To, is this work that someone on the OpenFE side would like to tackle? I am more than happy to assist but I think it might be a good exercise in knowledge transfer.

@IAlibay is the person to sync with on this

Co-authored-by: Alyssa Travitz <31974495+atravitz@users.noreply.github.com>
@ethanholz
Copy link
Contributor Author

We are in the home stretch, do we want to have @IAlibay take a look as well?

This means each ``ProtocolUnit``'s ``shared`` and ``permanent`` object are not paths, and should not be treated as such.
Both of these are registries that track if a file should be transferred from its location in ``scratch`` to its final location after completing a unit.

If you want to use some from ``shared`` or ``permanent``, you can use ``ctx.shared.load`` or ``ctx.permanent.load``.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If you want to use some from ``shared`` or ``permanent``, you can use ``ctx.shared.load`` or ``ctx.permanent.load``.
To access data from ``shared`` or ``permanent``, you can use ``ctx.shared.load`` or ``ctx.permanent.load``.

you were missing a word I think

@github-actions
Copy link

No API break detected ✅

@atravitz
Copy link
Contributor

We are in the home stretch, do we want to have @IAlibay take a look as well?

@IAlibay - you can review here, or pass on reviewing here and give your edits when evaluating migration things, up to you!

Copy link
Member

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

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

Some initial thoughts / questions ahead of tomorrow's meeting.

"result_path": result_path_final_location,
}

The example above showcases how
Copy link
Member

Choose a reason for hiding this comment

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

Incomplete sentence?

Copy link
Member

Choose a reason for hiding this comment

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

Bumping this.


.. code-block:: python

ctx.shared.register("myfile.dat")
Copy link
Member

Choose a reason for hiding this comment

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

What if your underlying code cannot handle Path like objects? For example, the multistate sampler stores the checkpoint file using a filename that is relative to the main storage path, so when you pass the file it's just checkpoint.chk.

Copy link
Member

Choose a reason for hiding this comment

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

How do I get the full path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So you can use registration as well, however we assume that everything is stored in scratch. Remember that we explicitly make it such that the registered files are not Path-like objects to ensure that people don't use them as paths. One thing we could do, is remove the requirement that everything is stored in scratch and then you can register any file to be transferred at the end of the run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Potentially add just dumping loaded objects as Pathlib objects.

``ExternalStorage`` Base Class
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The :class:`.ExternalStorage` abstract base class defines the interface that all storage implementations must provide. This class provides:
Copy link
Member

Choose a reason for hiding this comment

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

I can see a get_filename method, but what if I want the full path / operate on the Pathlib object (i.e. if I want to get the file extension or the parent directory)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ExternalStorage objects intentionally don't use Pathlib so that we can abstract over the storage medium. This allows execution engines the choice of how they want to store files (which might not always be a filesystem).

"result_path": result_path_final_location,
}

The example above showcases how
Copy link
Member

Choose a reason for hiding this comment

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

Bumping this.

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