Skip to content

Split-merge - #451

Merged
mihaic merged 10 commits into
brainiak:masterfrom
cbaldassano:splitmerge
Feb 6, 2020
Merged

mihaic merged 10 commits into
brainiak:masterfrom
cbaldassano:splitmerge

Conversation

@cbaldassano

Copy link
Copy Markdown
Collaborator

This pull request implements an improvement to the fitting procedure for the HMM, allowing for re-allocating events during fitting with a split-merge operation. This behavior is disabled by default so as not to break backwards compatibility and because it adds additional computation time, but can be enabled using the new split_merge flag in the constructor.

A draft of a blog post explaining the logic behind this change and some example fits can be found here: http://chrisbaldassano.com/Split-Merge.mhtml

@cbaldassano

cbaldassano commented Jan 14, 2020 •

Copy link
Copy Markdown
Collaborator Author

Jenkins is failing with "salloc: error: Job submit/allocate failed: Unable to contact slurm controller (connect failure)" - looking through my emails I think slurm is down for maintenance at Princeton today, can a maintainer please restart the checks once it is back up?

Edit: The macos build is failing with an error related to TFA: "Warning, treated as error:
invalid signature for automodule ('brainiak.factoranalysis.tfa_extension.cpython-37m-darwin')
make: *** [html] Error 2"

@cbaldassano
cbaldassano requested review from mihaic and qihongl January 14, 2020 15:40
@mihaic

mihaic commented Jan 14, 2020

Copy link
Copy Markdown
Member

@cbaldassano, thanks for the heads up. We are working of fixing the problems.

@qihongl, you can see on Travis that the unit tests are passing, so please disregard the failures and review as soon as possible.

@tristansyates

Copy link
Copy Markdown

I would be happy to review this! @cbaldassano @qihongl

Comment thread brainiak/eventseg/event.py Resolved
Comment thread brainiak/eventseg/event.py Resolved
Comment thread brainiak/eventseg/event.py Resolved
@cbaldassano

Copy link
Copy Markdown
Collaborator Author

Thanks @tsy6! @qihongl if you agree that this can be merged then I think it is ready to go - not sure what is causing the linux-specific build error, the error log seems to be related to some kind of compiler misconfiguration (unrelated to this code)

@mihaic

mihaic commented Feb 6, 2020

Copy link
Copy Markdown
Member

Indeed, the build error was an unrelated. Should be fixed.

@cbaldassano, could you please also write a blurb for the release notes? See this old one if you need to refresh your memory on how they work:
https://github.com/brainiak/brainiak/blob/fe604afbf1b919fb9fe4ce9c5e2c8bede94ec3ef/docs/newsfragments/353.feature

@cbaldassano

Copy link
Copy Markdown
Collaborator Author

Done!

@mihaic
mihaic merged commit 1385038 into brainiak:master Feb 6, 2020
@mihaic

mihaic commented Feb 6, 2020

Copy link
Copy Markdown
Member

Thank you for your contributions, @cbaldassano and @tsy6!

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