-
Notifications
You must be signed in to change notification settings - Fork 75
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
Added mean field protocol for complexes #223
base: develop
Are you sure you want to change the base?
Conversation
Great, thanks Kelly! I think it makes sense to consider some refactoring at this point to avoid code duplication - just like the PLM-based protocols have infer_plm(), the mean-field based protocols could have infer_meanfield() that contains most of the non-specific code shared between monomers and complexes etc. |
@thomashopf agree wholeheartedly - will better modularize after current research push :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good- there's only minor documentation and whitespace changes that need to be made. I've been using this protocol successfully with no issues.
[ | ||
"prefix", "min_sequence_distance", | ||
"scoring_model", "use_all_ecs_for_scoring", | ||
"alignment_file","segments","focus_mode", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
put spaces after commas in list
"prefix", "min_sequence_distance", | ||
"scoring_model", "use_all_ecs_for_scoring", | ||
"alignment_file","segments","focus_mode", | ||
"focus_sequence","theta","pseudo_count", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spaces
|
||
prefix = kwargs["prefix"] | ||
|
||
# infer ECs and load them with plmc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete these obsolete lines
|
||
#infer ECs with mean field/DCA | ||
|
||
# Below here is added material from mean_field protocol: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete commented line
#infer ECs with mean field/DCA | ||
|
||
# Below here is added material from mean_field protocol: | ||
model = prefix+".model" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefix + ".model"
) | ||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
limit to one whitespace line
if segments is not None: | ||
# create index mapping | ||
seg_mapper = mapping.SegmentIndexMapper( | ||
# kwargs["focus_mode"], outcfg["region_start"], *segments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete commented line
ecs = mapping.segment_map_ecs(ecs,seg_mapper) | ||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
limit to one whitespace line
# file_format="plmc_v2" | ||
# ) | ||
|
||
###### End added material |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete commented lines
This branch is included in #225 - ok to close this pull request? |
Does this already have the suggested refactoring to reduce code redundancy? |
No, there's been no refactoring that I know of |
Based directly off of the PLM code, but used MeanField model and fit. Note: for region_start when mapping segments, I used the number in kwargs instead of the one taken from the MeanField model - the latter led to incorrect numbering.