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

Update to HiddenMarkovModels v0.6 #9

Merged
merged 3 commits into from
Oct 25, 2024
Merged

Conversation

gdalle
Copy link
Contributor

@gdalle gdalle commented Oct 24, 2024

I recently released a breaking version of HiddenMarkovModels, so here's a small PR updating the compat bound and adding a few views in passing for speed

@dmetivie
Copy link
Owner

Thanks! I just up version and added your new AbstractVectorOrNTuple type for seq_ends.

@dmetivie dmetivie merged commit cfa322e into dmetivie:master Oct 25, 2024
@gdalle
Copy link
Contributor Author

gdalle commented Oct 25, 2024

Thanks! I just up version and added your new AbstractVectorOrNTuple type for seq_ends.

This is not documented so it is not part of the public API and could break in patch release. At the moment you should not rely on this functionality.

@gdalle gdalle deleted the update_hmms branch October 25, 2024 14:26
@gdalle
Copy link
Contributor Author

gdalle commented Oct 25, 2024

The main reason I introduced it was to activate or deactivate multithreading based on dispatch, so that a single sequence (passed as seq_ends=(T,)) would not trigger the overhead of multithreading. It is mostly an internal change.

@dmetivie
Copy link
Owner

Do you recommend removing the type annotation for seq_ends in StatsAPI.fit! function, then?
Keeping the old AbstractVector{Int} was making the code error.

@gdalle
Copy link
Contributor Author

gdalle commented Oct 25, 2024

I would recommend simply removing the annotation on seq_ends in your custom fit!, since now it must be able to accept both a vector and a tuple. I believe that's how I do it in the new docs but I'll gladly take suggestions to document this a bit better.

@gdalle
Copy link
Contributor Author

gdalle commented Oct 25, 2024

And lastly, you don't need to release a breaking version when you update a dependency like that, except when it has an impact on your package's own user-facing API. So here tagging v0.2.1 would probably have sufficed. But we can discuss it face to face in Toulouse ;)

@dmetivie
Copy link
Owner

Dam! I can always revert the pkg number since it not release yet?

Can't wait (I'll be there only Tuesday, unfortunately)!

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