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

Reorganize package content #194

Merged
merged 14 commits into from
Jan 30, 2023
Merged

Reorganize package content #194

merged 14 commits into from
Jan 30, 2023

Conversation

felixhekhorn
Copy link
Contributor

The idea is to split the hard math away from the other administrative stuff (such as how to join operators etc.). This would be a small step towards #185 and eventually #189.

@felixhekhorn felixhekhorn added refactor Refactor code benchmarks Benchmark (or infrastructure) related labels Jan 11, 2023
@alecandido
Copy link
Member

This will also simplify #185, eventually.
In theory, the scope of that PR would become to move ekore and only it to Rust (in practice we might need several further adjustments because of numba)

@alecandido
Copy link
Member

@felixhekhorn do you actually want a review now, or are you just attracting our attention on the topic?

@felixhekhorn
Copy link
Contributor Author

no, not yet - I want to attempt the move into unpolarized/space_like/* first ... I'll mark this PR

@alecandido
Copy link
Member

alecandido commented Jan 12, 2023

Maybe we should make a poll about how to sort external dimensions, currently:

  1. anom-dim vs matching
  2. polarization
  3. time vs space
  4. pto
  5. flavor

I'm positive about keeping 4 and 5 in their current position, encoding them in modules and functions respectively. But I'm quite unopinionated on the first three.

giacomomagni
giacomomagni previously approved these changes Jan 16, 2023
Copy link
Collaborator

@giacomomagni giacomomagni left a comment

Choose a reason for hiding this comment

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

Test are passing, so these changes should be enough :)

@felixhekhorn felixhekhorn marked this pull request as ready for review January 17, 2023 17:26
@felixhekhorn
Copy link
Contributor Author

  • the most content of this PR is just a moving of files and the associated renames
  • for the OME I had to alter a bit the structure to split the hard math away (which I called OME - I hope that is correct); @giacomomagni could you have a look please?
  • we can decide to lift the bunch of raise NotImplementedError into the calling functions inside eko to save us some docstrings ...

For review: please @alecandido @niclaurenti @giacomomagni have a look now

@alecandido alecandido changed the base branch from develop to master January 27, 2023 14:09
@alecandido alecandido dismissed giacomomagni’s stale review January 27, 2023 14:09

The base branch was changed.

@alecandido alecandido mentioned this pull request Jan 27, 2023
@alecandido alecandido force-pushed the split-math-in-module branch from 4c72a96 to 07aa331 Compare January 30, 2023 16:09
@felixhekhorn felixhekhorn merged commit 03efcb4 into master Jan 30, 2023
@felixhekhorn felixhekhorn deleted the split-math-in-module branch January 30, 2023 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmarks Benchmark (or infrastructure) related refactor Refactor code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants