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

Rustify ekore part 2: ad.u.s.{as2,as3} #369

Merged
merged 26 commits into from
May 30, 2024
Merged

Rustify ekore part 2: ad.u.s.{as2,as3} #369

merged 26 commits into from
May 30, 2024

Conversation

felixhekhorn
Copy link
Contributor

@felixhekhorn felixhekhorn commented May 13, 2024

Actually, we should push for the continued rustification before @giacomomagni receives his halo and someone else has to translate all those endless N3LO expression. I will not attempt the full rustification here but still proceed in parts (i.e. this is part 2 (after #189 ) out of X): currently my only goal here is to be ready for QCD N3LO ad. To be specific:

ekore/anomalous_dimensions/unpolarized/space_like/

  • as2.py
  • as3.py

@felixhekhorn felixhekhorn added refactor Refactor code rust Rust extension related labels May 13, 2024
This was referenced May 17, 2024
@giacomomagni
Copy link
Collaborator

Thanks @felixhekhorn
I'm not sure if I'll be able to contribute to this endeavour at some point

@tgiani
Copy link
Contributor

tgiani commented May 22, 2024

@felixhekhorn when you have time could you please have a look at as2.rs? It should be mostly done. I have not implemented the tests with NC=4 because I don t see how to change a const in rust, not sure if that can be done? If there are no major issues with what done in as2.rs I can proceed with as3.py

Edit: actually the qed stuff is not there. Should I add that as well or for the time being we go for qcd only?

@felixhekhorn
Copy link
Contributor Author

for the time being we go for qcd only

indeed - let's proceed in parts and first we do QCD only

One simple comment up front: please run pre-commit

Copy link
Contributor Author

@felixhekhorn felixhekhorn left a comment

Choose a reason for hiding this comment

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

  • I think the NC=4 case is artificial and we simply don't allow for this hypothetical feature any longer
  • of course we eventually have to hook the function in on the higher level

@felixhekhorn
Copy link
Contributor Author

(now somehow cargo fmt is broken for me and I don't know why since it was working fine the whole afternoon, so I might have broken pre-commit myself, but I will try to fix this tomorrow)

* of course we eventually have to hook the function in on the higher level

8c304b8 should have done this

However, there seems to be something fishy: if you rustify and then run poe lha -m "ffns and nlo and not sv" you will see that V, V3 and T3 are completely off (by ~5%)

@tgiani
Copy link
Contributor

tgiani commented May 23, 2024

mm ok. I guess by rustify you mean running the script rustify.sh?

@felixhekhorn
Copy link
Contributor Author

mm ok. I guess by rustify you mean running the script rustify.sh?

exactly - this will hook the Rust backend into the main eko package + remember to actually compile via poe compile; this way we can run the usual eko workflows (with the limited available orders of course) e.g. to check against the LHA benchmark number which we will have to reproduce ...

@tgiani
Copy link
Contributor

tgiani commented May 23, 2024

ok I see. Other question: isn t pytest installed automatically in the poetry venv when I run poetry install?
If I do poetry install, rustify, poe compile and then poe lha -m "ffns and nlo and not sv" it doesn t find pytest

@felixhekhorn
Copy link
Contributor Author

ok I see. Other question: isn t pytest installed automatically in the poetry venv when I run poetry install? If I do poetry install, rustify, poe compile and then poe lha -m "ffns and nlo and not sv" it doesn t find pytest

no, since it is a developer dependency - you can do poetry install --with test (Poetry has this dependency groups)

@felixhekhorn
Copy link
Contributor Author

now somehow cargo fmt is broken for me and I don't know why since it was working fine the whole afternoon, so I might have broken pre-commit myself, but I will try to fix this tomorrow

okay, so it turns out that it was just a veeery long run for cargo fmt, but eventually it does terminate - as a fix I inserted a bunch of #[rustfmt::skip] - not all of them are actually required, but mostly the ones for the long expressions. However, then for consistency I put them all. In the end the expressions are just math and there is no big insight in having them formatted (although they are easier to check this way). What do you think @tgiani? should we keep the markers?

@tgiani
Copy link
Contributor

tgiani commented May 23, 2024

yea I think so else every time we commit we have to wait half an hour

@tgiani
Copy link
Contributor

tgiani commented May 23, 2024

I m trying to run the benchmark poe lha -m "ffns and nlo and not sv" to investigate the problem you mentioned yesterday but that also has been run for several minutes now. Do you know how long it is supposed to take?

@felixhekhorn
Copy link
Contributor Author

felixhekhorn commented May 23, 2024

I m trying to run the benchmark poe lha -m "ffns and nlo and not sv" to investigate the problem you mentioned yesterday but that also has been run for several minutes now. Do you know how long it is supposed to take?

Ehm, yes this is a feature 🙃 (one that we are trying to partly tackle here) for the first run eko will take a while, because it needs to compile all the numba stuff, which is a lot. Normally it can take up to 15 min if the OME are involved, but actually here it should be quicker, since we're running FFNS and poe compile already did part of the work. You can check if numba is the problem by setting NUMBA_DEBUG or you can set EKO_LOG_STDOUT to check what EKO is doing.

PS: the second run is ~10s for me

@tgiani
Copy link
Contributor

tgiani commented May 23, 2024

Ehm, yes this is a feature 🙃 (one that we are trying to partly tackle here) for the first run eko will take a while, because it needs to compile all the numba stuff, which is a lot. Normally it can take up to 15 min if the OME are involved, but actually here it should be quicker, since we're running FFNS and poe compile already did part of the work. You can check if numba is the problem by setting NUMBA_DEBUG or you can set EKO_LOG_STDOUT to check what EKO is doing.

PS: the second run is ~10s for me

ok after 30 min I ve stopped it and I m trying again setting the two env variables you suggested. Is the singlet sector alright from the benchmark? If so the problem should be something in gamma_nsm no?

@felixhekhorn
Copy link
Contributor Author

ok after 30 min I ve stopped it and I m trying again setting the two env variables you suggested. Is the singlet sector alright from the benchmark? If so the problem should be something in gamma_nsm no?

Mmm not sure ... in the singlet sector we match my 0.4% and I'm not sure if that is good enough (LO we match by 0.02%) and we should check with master. I also see that T8 and T15 are kind of okay, but T3 is not - and all should be NS+ ... Of course T3 is small in absolute value, so maybe there is some precision loss somewhere?

@tgiani
Copy link
Contributor

tgiani commented May 23, 2024

mm i don t know... In the expressions I m doing the following replacements:

  • I turn all integers except those in exponents into floats (else algebra with complex is not defined)
  • np.power(N,integer)->N.powu(integer)
  • np.pi**2 -> PI.pow(2)
  • NF -> (NF as f64)

does any of this seem wrong?

@tgiani
Copy link
Contributor

tgiani commented May 23, 2024

anyway I m still stuck on this poe lha -m "ffns and nlo and not sv". Since about 1hr 30 min the last log I get from numba is

================================================================================
-------------------------OPTIMIZED DUMP cb_quad_ker_qcd-------------------------
; ModuleID = 'cb_quad_ker_qcd'
source_filename = "<string>"
target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
target triple = "arm64-apple-darwin23.4.0"

================================================================================
================================================================================
----------------------------ASSEMBLY cb_quad_ker_qcd----------------------------
        .section        __TEXT,__text,regular,pure_instructions
        .build_version macos, 14, 0
.subsections_via_symbols

================================================================================

not sure what the problem here

@felixhekhorn felixhekhorn marked this pull request as ready for review May 29, 2024 14:08
@felixhekhorn
Copy link
Contributor Author

c11f6c4 fixes a stupid typo which was preventing us from passing the benchmark - now we are passing as before. If there are no complaints I will merge this tomorrow.

@tgiani c11f6c4 also added an explicit nopython argument to the function which is passed through Rust - this should already be the default (this changed in numba 0.59), but can you please check nevertheless that you still have the same exception?

@tgiani
Copy link
Contributor

tgiani commented May 29, 2024

@felixhekhorn ah ok good. Yes also after your last commit I still have the same problem on mac

@felixhekhorn
Copy link
Contributor Author

felixhekhorn commented May 29, 2024

Continuing from here we can have several parallel parts:

  1. ome.u.s.as1 and ome.u.s.as2 + VFNS
  2. recover SV
  3. ad.p.s.*
  4. recover solution methods
  5. ad.u.s.as4 and ad.u.s.as4.fhmruvv
  6. ad.u.s.*aem* + QED
  7. ad.u.t.*
  8. ome.u.s.as4
  9. anything left

The order of the list is a slight indication of priority. Not every item needs a separate PR although that might help in the review. In case you are not tired yet @tgiani pick your favourite!

EDIT: see #383

@felixhekhorn felixhekhorn changed the title Rustify ekore part 2 Rustify ekore part 2: ad.u.s.{as2,as3} May 29, 2024
@tgiani
Copy link
Contributor

tgiani commented May 29, 2024

sure, I guess I ll start from 1. What s 2?

@felixhekhorn
Copy link
Contributor Author

What s 2?

actually quite simple:

sv_mode = sv.Modes.exponentiated

replace the hardcoded value by reading from _sv_mode_num and put that parameter to a sensible value in the first place
+ cfg.sv_mode_num = 1

the same holds for item 4 (e.g. they could go in one PR)

@felixhekhorn felixhekhorn mentioned this pull request May 29, 2024
11 tasks
@felixhekhorn felixhekhorn merged commit ca7112f into master May 30, 2024
8 checks passed
@felixhekhorn felixhekhorn deleted the rustify-ekore-2 branch May 30, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactor code rust Rust extension related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants