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

Unified API #8

Merged
merged 8 commits into from
Jan 30, 2024
Merged

Unified API #8

merged 8 commits into from
Jan 30, 2024

Conversation

SamDuffield
Copy link
Contributor

This PR provides a unified API for uqlib methods.

The unified API is optax/BlackJAX style with init, update and build functions:

# define log_posterior, params and dataloader

transform = uqlib_method.build(log_posterior, **config_args)

state = transform.init(params)

for batch in dataloader:
       state = transform.update(state, batch)

examples/lightning_autoencoder.py has a full example of how to use the API with pytorch lightning in a way that you can seamlessly swap between sghmc and vi.diag.

@SamDuffield SamDuffield marked this pull request as draft January 23, 2024 18:07
@SamDuffield SamDuffield marked this pull request as ready for review January 24, 2024 16:25
batch,
inplace=False,
)
for key in expected:

Choose a reason for hiding this comment

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

i don't get what this little bit is doing down here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just wanted to test that inplace=True indeed modifies the input state in-place, whilst inplace=False leaves the input state untouched.

Definitely this could be improved, although perhaps in a later PR

) -> TransformState:
"""Initiate a state.

Args:

Choose a reason for hiding this comment

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

explain this a bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lol yep definitely

Copy link

@dmitrisaberi dmitrisaberi left a comment

Choose a reason for hiding this comment

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

LGTM, just left some clarifying questions!

@SamDuffield SamDuffield merged commit a08c55d into main Jan 30, 2024
2 checks passed
@SamDuffield SamDuffield deleted the unified-api branch January 30, 2024 11:56
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