Generate callables for each PTO #123
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #123 +/- ##
============================================
- Coverage 100.00% 53.41% -46.59%
============================================
Files 67 68 +1
Lines 3214 3263 +49
============================================
- Hits 3214 1743 -1471
- Misses 0 1520 +1520
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Actually, I think |
unfortunately in practice this is visible: for the first step in matching we pay 20s - compared to the ~7-8s for the second step; but for the moment I don't see a way around ... |
The way out is to use the generator to actually write the function in a file, such that there is a given address. But I'm not sure that we want to do it, playing with files might have a long tail... |
|
@alecandido this is my second attempt; I'm not too happy, because there is too much copy and paste still in the game ... to be specific: these 11 lines https://github.com/N3PDF/eko/blob/61068792c2c53cc9d719dfd054fde4d769af14f8/src/eko/matching_conditions/operator_matrix_element.py#L119-L135 are copied almost exact 3 times ... I guess, I should start introducing function pointers ... what do you think? |
|
Wrap the three assignments into a single function: Also these two lines can be packed together in a single function: You know, they are small, so you might wonder if it's worth to bundle them into a function. I tell you that is worth: if you need to change, a single place will be affected. In this way you go down to 8 lines. And I know they are repeated, but code is not only measured in lines: they contain almost nothing, the differences are relevant. Factorize what is simple to gather, keep explicit everything else. We are always in time to further improve. |
| return ker[indices[self.mode0], indices[self.mode1]] | ||
| # AD mode | ||
| k = 0 if self.mode0 == 100 else 1 | ||
| l = 0 if self.mode1 == 100 else 1 |
There was a problem hiding this comment.
I had a look at code factor, and most of the remaining issues are with poor names.
On one side, the immediate feeling is that it is too picky, and for sure it is not able to recognize long living variables from local indices and so on. But on the other, it is only (or mostly) complaining about one letter variables, that we might really consider too little, since it is for sure not meaningful at all (almost anonymous).
Here, e.g., I would make the minimal effort to improve, and just call the two of them fl1 and fl2 (or fli and flo, for I/O).
|
Should we close this? |
At some point, definitely. But @felixhekhorn mentioned that he wanted to resume the project, when possible. We can close and restart from scratch later on, or wait for the replacement to close. |
yes, with more and more math coming in (QED, N3LO, polarized, FF), i.e. more and more complicated compilation, I consider this will become relevant again.
since a lot of things changed meanwhile, I guess we can convert the idea into an issue (mentioning the alternative proposal of using C/Rust) and close this PR ... |
|
This is outdated, and by now on hold. Since we might want to step out Numba, maybe it will also be solved in a different way. |
|
Shifted as issue in #204 |
Closes #119
nbwants a definitive address (which we haven't since we generate on the fly); however, this is maybe acceptable since they are short and the hard work (the actual elements) are fully cachedwe should assign this some priority to make the package usable again from the outside