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 CLI #171

Closed
wants to merge 18 commits into from
Closed

Reorganize CLI #171

wants to merge 18 commits into from

Conversation

alecandido
Copy link
Member

@alecandido alecandido commented Dec 3, 2022

The EKO CLI was already initialized, but completely forgotten and unused.

I will add some basic commands to generate EKOs, useful to test further developments.

  • conversion subcommand

@alecandido alecandido changed the base branch from master to output_metadata December 3, 2022 16:13
@codecov-commenter
Copy link

codecov-commenter commented Dec 3, 2022

Codecov Report

Merging #171 (0b5372e) into output_metadata (f01fc19) will decrease coverage by 0.50%.
The diff coverage is 83.91%.

Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##           output_metadata     #171      +/-   ##
===================================================
- Coverage            99.93%   99.43%   -0.51%     
===================================================
  Files                   96      100       +4     
  Lines                 4515     4586      +71     
===================================================
+ Hits                  4512     4560      +48     
- Misses                   3       26      +23     
Flag Coverage Δ
isobench 53.64% <29.37%> (-0.87%) ⬇️
unittests 99.43% <83.91%> (-0.51%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/ekobox/cli/run.py 45.45% <40.00%> (-54.55%) ⬇️
src/ekobox/cli/convert.py 69.23% <69.23%> (ø)
src/ekobox/cli/runcards.py 78.94% <78.94%> (ø)
src/ekobox/cli/inspect.py 85.00% <85.00%> (ø)
src/eko/output/legacy.py 98.43% <100.00%> (ø)
src/eko/output/struct.py 99.72% <100.00%> (+<0.01%) ⬆️
src/ekobox/__init__.py 100.00% <100.00%> (ø)
src/ekobox/cards.py 100.00% <100.00%> (ø)
src/ekobox/cli/__init__.py 100.00% <100.00%> (ø)
src/ekobox/cli/base.py 100.00% <100.00%> (ø)
... and 2 more

@felixhekhorn felixhekhorn added enhancement New feature or request output Output format and management labels Dec 5, 2022
@alecandido
Copy link
Member Author

Docs can be improved, there is no coverage for the new commands.

However, not my priority at the moment, so I will stop working on this here for a while, and use for my next PR.
If someone wants to merge sooner, at least test coverage should be recovered.

@alecandido alecandido requested review from felixhekhorn and andreab1997 and removed request for felixhekhorn December 5, 2022 17:57
Copy link
Contributor

@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 added two v0.8.5 example ekos we for sure want to support given that most of our "real-life" ekos are of this version - both fail:

  • the yaml due to the problem given below
  • the other with
$ poetry run eko convert obf24af_t8e1305.tar
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/felix/.cache/pypoetry/virtualenvs/eko-KkPVjVhh-py3.10/lib/python3.10/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/home/felix/.cache/pypoetry/virtualenvs/eko-KkPVjVhh-py3.10/lib/python3.10/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/home/felix/.cache/pypoetry/virtualenvs/eko-KkPVjVhh-py3.10/lib/python3.10/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/felix/.cache/pypoetry/virtualenvs/eko-KkPVjVhh-py3.10/lib/python3.10/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/felix/.cache/pypoetry/virtualenvs/eko-KkPVjVhh-py3.10/lib/python3.10/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/home/felix/Physik/N3PDF/EKO/eko/src/ekobox/cli/convert.py", line 27, in subcommand
    operator = legacy.load_tar(old)
  File "/home/felix/Physik/N3PDF/EKO/eko/src/eko/output/legacy.py", line 278, in load_tar
    metadata = tocard(yaml.safe_load(fd))
  File "/home/felix/Physik/N3PDF/EKO/eko/src/eko/output/legacy.py", line 84, in tocard
    del card["pids"]
KeyError: 'pids'

@felixhekhorn
Copy link
Contributor

felixhekhorn commented Dec 6, 2022

Also benchmarks are broken:

$ poe lha
Poe => python benchmarks/lha_paper_bench.py
 ──────────────────────────────────────── 
  Theories: 1 OCards: 1 PDFs: 1 ext: LHA  
 ──────────────────────────────────────── 
Computing for theory=fb3f558, ocard=7f27498 and pdf=ToyLH ...
Traceback (most recent call last):
  File "/home/felix/Physik/N3PDF/EKO/eko/benchmarks/lha_paper_bench.py", line 226, in <module>
    obj.benchmark_plain(0)
  File "/home/felix/Physik/N3PDF/EKO/eko/benchmarks/lha_paper_bench.py", line 123, in benchmark_plain
    self.run_lha(self.plain_theory(pto))
  File "/home/felix/Physik/N3PDF/EKO/eko/benchmarks/lha_paper_bench.py", line 109, in run_lha
    self.run(
  File "/home/felix/.cache/pypoetry/virtualenvs/eko-KkPVjVhh-py3.10/lib/python3.10/site-packages/banana/benchmark/runner.py", line 416, in run
    self.run_config(session, t, o, pdf_name, use_replicas)
  File "/home/felix/.cache/pypoetry/virtualenvs/eko-KkPVjVhh-py3.10/lib/python3.10/site-packages/banana/benchmark/runner.py", line 304, in run_config
    me = self.run_me(t, o, pdf)
  File "/home/felix/Physik/N3PDF/EKO/eko/src/ekomark/benchmark/runner.py", line 119, in run_me
    out = eko.run_dglap(theory, ocard)
  File "/home/felix/Physik/N3PDF/EKO/eko/src/eko/__init__.py", line 29, in run_dglap
    r = runner.Runner(theory_card, operators_card)
  File "/home/felix/Physik/N3PDF/EKO/eko/src/eko/runner.py", line 50, in __init__
    new_theory, new_operators = compatibility.update(theory_card, operators_card)
  File "/home/felix/Physik/N3PDF/EKO/eko/src/eko/compatibility.py", line 57, in update
    new_operators["rotations"]["pids"] = operators["pids"]
KeyError: 'pids'

EDIT: sorry is already a problem in the base branch ... I'll raise there

@felixhekhorn felixhekhorn mentioned this pull request Dec 6, 2022
This was referenced Dec 7, 2022
@alecandido alecandido closed this Dec 14, 2022
@felixhekhorn felixhekhorn deleted the cli branch January 5, 2023 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request output Output format and management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants