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

ties fix with unit test #24

Merged
merged 2 commits into from
Apr 24, 2024
Merged

ties fix with unit test #24

merged 2 commits into from
Apr 24, 2024

Conversation

6DammK9
Copy link
Contributor

@6DammK9 6DammK9 commented Apr 23, 2024

As discussed.
Added comments and unit test, meanwhile eliminated all loops (syntax trick only).
vmap is only for batched processing. Data type was messed.
Yet it is a merge with O(N) space complexity.

Copy link
Owner

@ljleb ljleb left a comment

Choose a reason for hiding this comment

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

thanks for the contribution, generally I'm fine with annotating with pseudo code from paper for more clarity. I primarily like functions to fit on my screen, but here I think you're right that matching the paper implementation is more important.

Let me know if you're okay with me pushing to this branch, I can tackle the comments I left in this case.

I'll close #23 in favor of this.

sd_mecha/merge_methods.py Outdated Show resolved Hide resolved
examples/unit_test/unit_test_ties.py Outdated Show resolved Hide resolved
examples/unit_test/unit_test_ties.py Outdated Show resolved Hide resolved
@ljleb
Copy link
Owner

ljleb commented Apr 24, 2024

I made some changes to the format, pycharm was not happy with triple quotes (giving warnings) so I replaced with comments. Let me know if this doesn't work with the vscode extension, in which case I'll change my formatting settings for this project I guess.

@6DammK9
Copy link
Contributor Author

6DammK9 commented Apr 24, 2024

Native VSCode don't have LaTeX stuffs. I think upmath.me / Jupyter notebook / github are all not compatable each other.
Especially github, it is in single $ without space e.g. $\alpha$ for $\alpha$
Usually I need to edit again copying them across platforms.
It will be fine when they can be formatted in any one platform. It is fine for me.
Just pick one and keep it consistent.

@ljleb ljleb merged commit f9af9e4 into ljleb:main Apr 24, 2024
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