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

Add replicas #36

Merged
merged 15 commits into from
Jun 26, 2024
Merged

Add replicas #36

merged 15 commits into from
Jun 26, 2024

Conversation

EzryStIago
Copy link
Collaborator

@EzryStIago EzryStIago commented Feb 21, 2024

Closes issue #8 : U_nk save/load
Closes issue #32 : replicas

@EzryStIago EzryStIago linked an issue Feb 21, 2024 that may be closed by this pull request
Copy link
Collaborator

@jhenin jhenin 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! I've changed the variable name filename_regex for precision, because a glob pattern is not the same as a regex.
To test the replica code, I've replicated the sample data trivially using symlinks. It's working as expected, except one thing I don't understand on the per-window deltaG curve:
image

What are these gray lines that deviate from the main data?

@EzryStIago
Copy link
Collaborator Author

What are these gray lines that deviate from the main data?

@jhenin Those are the forward and backward exponential estimates. They look gray because each replica gets alpha<1 and a different color. I'll have to think about a better way to present this. Maybe we don't really need the exponential estimates? Or we could make them a flag.

@jhenin
Copy link
Collaborator

jhenin commented Feb 28, 2024

I see. I think these plots are useful, but they should be documented, probably in a legend.

@EzryStIago
Copy link
Collaborator Author

I've added a legend to that plot:
image

@EzryStIago EzryStIago requested a review from jhenin June 13, 2024 15:34
Copy link
Collaborator

@jhenin jhenin left a comment

Choose a reason for hiding this comment

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

With a minor fix that I just pushed, this works well.
Ideally, a sample dataset with different data in the replicas would be nice to have, but it's not a show-stopper.

@EzryStIago EzryStIago merged commit caa2a3a into main Jun 26, 2024
@EzryStIago EzryStIago deleted the add_replicas branch June 26, 2024 14:35
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.

TODO Add multi-dataset BAR processing notebook
2 participants