Skip to content

speedup isless productsector #3

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

Merged
merged 6 commits into from
Oct 29, 2024
Merged

speedup isless productsector #3

merged 6 commits into from
Oct 29, 2024

Conversation

Jutho
Copy link
Member

@Jutho Jutho commented Oct 28, 2024

Rather than computing the Manhattan index for defining isless, use that the Manhattan index is increasing in such a way that multidimensional indices are sorted first according to total manhattan distance to the origin, then among those with fixed total manhattan distance, sorted according to manhattan distance of the first N-1 indices, and so forth.

@Jutho
Copy link
Member Author

Jutho commented Oct 29, 2024

Actually, it is simpler than that; there is no such recursive structure. The order is just in terms of increasing total Manhattan distance, and within the same Manhattan distance it is simply lexographic ordering of the multidimensional index.

Copy link
Member

@lkdvos lkdvos left a comment

Choose a reason for hiding this comment

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

I'm not sure I fully understand the implementation, but that sounds reasonable to me 🙃

Copy link

codecov bot commented Oct 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
src/auxiliary.jl 92.42% <100.00%> (ø)
src/irreps.jl 95.43% <100.00%> (ø)
src/product.jl 82.58% <100.00%> (+0.60%) ⬆️

@Jutho
Copy link
Member Author

Jutho commented Oct 29, 2024

I am not quite sure why JET is complaining on Julia 1.11.1. I won't have much time to look into it until later today.

@Jutho
Copy link
Member Author

Jutho commented Oct 29, 2024

Ok I fixed the JET complaints by adding some type restrictions/specialisations. It seems very un-Julian that this is necessary, but I guess that for now it is without harm.

@Jutho
Copy link
Member Author

Jutho commented Oct 29, 2024

Also, I don't know why Julia 1.10 ubuntu-latest x64 CI is not running/reporting. Is this some rule you set up @lkdvos ?

@Jutho Jutho merged commit 3e6ce25 into main Oct 29, 2024
8 checks passed
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