Skip to content

Handle nesting for ConvertDType, ToArray, adapt concatenate dispatch #503

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

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

vpratz
Copy link
Collaborator

@vpratz vpratz commented Jun 1, 2025

This PR generalizes the transforms from the Approximator.create_default method, so that they can be used with nested inputs as well. For ConvertDType and ToArray, it supplies a generalization that works on dictionaries.
Edit: For Concatenate, it adds a minor change in the Adapter.concatenate dispatch function to detect the special case of only one key being present also if it is nested in a sequence of length one.

The recursive structure of nested inputs requires some extra utility functions, which I put into bayesflow.utils.tree.

Concatenate can be equal to rename if only one key is supplied. By not
calling concatenate in that case, we can accept arbitrary inputs in the
transform, as long as only one is supplied. This simplifies things e.g.
in the `BasicWorkflow`, where the user passes the `summary_variables` to
concatenate, which may be a single dict, which does not need to be
concatenated.
Copy link

codecov bot commented Jun 1, 2025

Codecov Report

Attention: Patch coverage is 74.57627% with 15 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
bayesflow/utils/tree.py 63.63% 12 Missing ⚠️
bayesflow/adapters/transforms/to_array.py 85.00% 3 Missing ⚠️
Files with missing lines Coverage Δ
bayesflow/adapters/adapter.py 85.54% <100.00%> (+0.11%) ⬆️
bayesflow/adapters/transforms/convert_dtype.py 100.00% <100.00%> (ø)
bayesflow/utils/__init__.py 100.00% <100.00%> (ø)
bayesflow/adapters/transforms/to_array.py 80.95% <85.00%> (+1.78%) ⬆️
bayesflow/utils/tree.py 63.63% <63.63%> (ø)

... and 2 files with indirect coverage changes

@vpratz vpratz requested review from LarsKue and stefanradev93 June 2, 2025 11:53
@LarsKue
Copy link
Contributor

LarsKue commented Jun 5, 2025

@vpratz Isn't the concat issue already addressed by this if-else in the adapter dispatch?

if isinstance(keys, str):
transform = Rename(keys, to_key=into)

@vpratz
Copy link
Collaborator Author

vpratz commented Jun 5, 2025

Thanks for the response. I didn't see that, I encountered an error in the transform, probably because I passed a list of length one instead of a string. With that additional context, I think it's better to just add that case in the dispatch as well, and revert the changes in the transform.

@vpratz vpratz changed the title Handle nesting for ConvertDType, ToArray. Relax conditions for Concatenate if only one key is present Handle nesting for ConvertDType, ToArray, adapt concatenate dispatch Jun 7, 2025
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.

2 participants