Skip to content

adds resource list script#764

Merged
Zaharid merged 9 commits into
masterfrom
add-vp-list
May 18, 2020
Merged

adds resource list script#764
Zaharid merged 9 commits into
masterfrom
add-vp-list

Conversation

@wilsonmr
Copy link
Copy Markdown
Contributor

@wilsonmr wilsonmr commented May 6, 2020

first attempt at creating script mentioned in #735

can update the docs if this seems like a sensible thing

I'm sure there should be a way to either take the positional argument resource or --list but I couldn't work out a way of doing it with mutuall exclusive groups, input welcome

@wilsonmr wilsonmr requested review from Zaharid and voisey May 6, 2020 13:02
@wilsonmr
Copy link
Copy Markdown
Contributor Author

wilsonmr commented May 6, 2020

@voisey I think this will be easier than having to check the server manually, let me know what you think

@wilsonmr
Copy link
Copy Markdown
Contributor Author

wilsonmr commented May 6, 2020

could potentially add the following features:

  • check if any local resources contain "deleteme" and suggest removing

I also noticed there is a theory with an empty string name which looks like a broken upload or something, perhaps we should remove that?

'': 'https://nnpdf.web.cern.ch/nnpdf/tables/.sys.v#.theory_189.tgz'

Comment thread validphys2/src/validphys/scripts/vp_list.py Outdated
Comment thread validphys2/src/validphys/scripts/vp_list.py Outdated
Comment thread validphys2/src/validphys/scripts/vp_list.py Outdated
@Zaharid
Copy link
Copy Markdown
Contributor

Zaharid commented May 8, 2020

Would it make sense to have a remote only option?

@wilsonmr
Copy link
Copy Markdown
Contributor Author

wilsonmr commented May 8, 2020

yeah I added both remote and local options and updated docs. Thoughts?

Comment thread doc/sphinx/source/tutorials/list-fits.md
Comment thread validphys2/src/validphys/scripts/vp_list.py Outdated
Comment thread validphys2/src/validphys/scripts/vp_list.py Outdated
- pdfs
- fits
- datasets
- theories
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Something seems to go wrong when I do vp-list theories -r. I get:

-
- 2
- 3
- 52
- 53
- 64
- 65
- 66
- 67
...

so firstly the first item isn't rendered properly and then lots of indexes, for instance between 3 and 52, are missing. Do you get something similar?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

see my comment about there being a theory whos ID ends up being an empty string

I also see that list, like I said I'm just printing what validphys sees. If I go to where we keep our theories then the list is consistent with the available files

[nnpdf@lxplus7126 www]$ ls tables/
114.files       theory_109.tgz  theory_130.tgz  theory_141.tgz  theory_152.tgz  theory_163.tgz  theory_174.tgz  theory_193.tgz  theory_206.tgz  theory_67.tgz  theory_81.tgz  theory_92.tgz
53.files        theory_110.tgz  theory_131.tgz  theory_142.tgz  theory_153.tgz  theory_164.tgz  theory_175.tgz  theory_194.tgz  theory_207.tgz  theory_71.tgz  theory_82.tgz  theory_93.tgz
theory_100.tgz  theory_111.tgz  theory_132.tgz  theory_143.tgz  theory_154.tgz  theory_165.tgz  theory_176.tgz  theory_195.tgz  theory_208.tgz  theory_72.tgz  theory_83.tgz  theory_94.tgz
theory_101.tgz  theory_112.tgz  theory_133.tgz  theory_144.tgz  theory_155.tgz  theory_166.tgz  theory_177.tgz  theory_196.tgz  theory_209.tgz  theory_73.tgz  theory_84.tgz  theory_99.tgz
theory_102.tgz  theory_113.tgz  theory_134.tgz  theory_145.tgz  theory_156.tgz  theory_167.tgz  theory_178.tgz  theory_197.tgz  theory_2.tgz    theory_74.tgz  theory_85.tgz  theorydata.json
theory_103.tgz  theory_114.tgz  theory_135.tgz  theory_146.tgz  theory_157.tgz  theory_168.tgz  theory_179.tgz  theory_199.tgz  theory_3.tgz    theory_75.tgz  theory_86.tgz  xx.py
theory_104.tgz  theory_115.tgz  theory_136.tgz  theory_147.tgz  theory_158.tgz  theory_169.tgz  theory_180.tgz  theory_200.tgz  theory_52.tgz   theory_76.tgz  theory_87.tgz
theory_105.tgz  theory_116.tgz  theory_137.tgz  theory_148.tgz  theory_159.tgz  theory_170.tgz  theory_181.tgz  theory_202.tgz  theory_53.tgz   theory_77.tgz  theory_88.tgz
theory_106.tgz  theory_117.tgz  theory_138.tgz  theory_149.tgz  theory_160.tgz  theory_171.tgz  theory_190.tgz  theory_203.tgz  theory_64.tgz   theory_78.tgz  theory_89.tgz
theory_107.tgz  theory_128.tgz  theory_139.tgz  theory_150.tgz  theory_161.tgz  theory_172.tgz  theory_191.tgz  theory_204.tgz  theory_65.tgz   theory_79.tgz  theory_90.tgz
theory_108.tgz  theory_129.tgz  theory_140.tgz  theory_151.tgz  theory_162.tgz  theory_173.tgz  theory_192.tgz  theory_205.tgz  theory_66.tgz   theory_80.tgz  theory_91.tgz

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I seeee. Sorry if this is obvious but why are lots of theories that are described here https://docs.nnpdf.science/theory/theoryindex.html not in that folder?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

do you want to open an issue about this? I think the actual script is working correctly in this regard it's just that there are some funny things about the server like missing theories and these weird directories like .sys.v#.theory_181.tgz which look like they are failed uploads or something

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

because I don't know the answer haha

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, sure 😛this obviously isn't a problem with this PR, it's just strange

```bash
$ vp-list --list
You can check the availability (locally and remotely) of the following resources:
- pdfs
Copy link
Copy Markdown
Contributor

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'm getting the desired behaviour for vp-list pdfs -l. I get:

[INFO]: The following pdfs are available locally
- LHAPDF

when my share/LHAPDF/ folder contains several symlinked pdf sets

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

is that the full output or is it for a subset of local pdfs? I am just using the function validphys uses to see available pdf sets and so I'm wondering if those sets even work with validphys?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You're right that validphys didn't know that these PDF sets existed. Perhaps they were linked to fits that I deleted or something like that. Anyway, I ran a short runcard with one of the sets that was meant to be there and vp downloaded it. Now the command gives:

[INFO]: The following pdfs are available locally:
- LHAPDF
- NNPDF31_nnlo_as_0118_DISonly

What does 'LHAPDF' being in this list mean? Do we expect it to be there in the list of PDFs?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Honestly it sounds like a problem on your end with the symlinks you've created

it's using

    def available_pdfs(self):
        return lhaindex.expand_local_names('*')

so either lhapdf doesn't know how to resolve a symlink or perhaps you still have in share/LHAPDF a file called LHAPDF which is either pointing at or supposed to be pointing at another LHAPDF folder somewhere. What is the result when you do:

$ ls $CONDA_PREFIX/share/LHAPDF

and if there is an LHAPDF there can you actually follow that link or is it broken?

I don't see this but then I never symlinked LHAPDF folders

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ls $CONDA_PREFIX/share/LHAPDF gives

lhapdf.conf   pdfsets.index

Note that I didn't create any of those symlinks myself. They were all generated automatically by validphys when a fit was downloaded

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I really don't know.
Out of interest what happens if you try and use LHAPDF in valipdhys, like

pdfs:
 - LHAPDF
Q: 2
actions_:
 - plot_pdfs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(nnpdf-dev) ➜  validphys2 git:(add-vp-list) ✗ validphys add-vp-list-test.yaml
[WARNING]: Output folder exists: /Users/CameronVoisey/nnpdfgit/nnpdf/validphys2/output Overwriting contents

----

Traceback (most recent call last):
  File "/Users/CameronVoisey/nnpdfgit/nnpdf/validphys2/src/validphys/core.py", line 102, in __getattr__
    return lhaindex.parse_info(self.name)[attr]
  File "/Users/CameronVoisey/nnpdfgit/nnpdf/validphys2/src/validphys/lhaindex.py", line 114, in parse_info
    with open(infofilename(name)) as infofile:
  File "/Users/CameronVoisey/nnpdfgit/nnpdf/validphys2/src/validphys/lhaindex.py", line 110, in infofilename
    raise FileNotFoundError(name + ".info")
FileNotFoundError: LHAPDF.info

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/bin/validphys", line 11, in <module>
    load_entry_point('validphys', 'console_scripts', 'validphys')()
  File "/Users/CameronVoisey/nnpdfgit/nnpdf/validphys2/src/validphys/scripts/main.py", line 10, in main
    vp.main()
  File "/Users/CameronVoisey/nnpdfgit/nnpdf/validphys2/src/validphys/app.py", line 150, in main
    a.main()
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/app.py", line 376, in main
    self.run()
  File "/Users/CameronVoisey/nnpdfgit/nnpdf/validphys2/src/validphys/app.py", line 146, in run
    super().run()
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/app.py", line 342, in run
    rb.resolve_fuzzytargets()
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/resourcebuilder.py", line 370, in resolve_fuzzytargets
    self.resolve_fuzzytarget(target)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/resourcebuilder.py", line 379, in resolve_fuzzytarget
    self.process_targetspec(fuzzytarget.name, spec, fuzzytarget.extraargs)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/resourcebuilder.py", line 388, in process_targetspec
    gen.send(None)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/resourcebuilder.py", line 445, in _process_requirement
    yield from self._make_node(name, nsspec, extraargs, parents)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/resourcebuilder.py", line 461, in _make_node
    yield from self._make_callspec(f, name, nsspec, extraargs, parents)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/resourcebuilder.py", line 524, in _make_callspec
    gen.send(cs)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/resourcebuilder.py", line 427, in _process_requirement
    yield from self._make_node((name, val.value), nsspec, extraargs, parents)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/resourcebuilder.py", line 457, in _make_node
    yield from self._make_collect_targets(f, name, nsspec, parents)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/resourcebuilder.py", line 666, in _make_collect_targets
    walk(colltargets.root, nsspec)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/resourcebuilder.py", line 651, in walk
    index, tnode = gen.send(None)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/resourcebuilder.py", line 445, in _process_requirement
    yield from self._make_node(name, nsspec, extraargs, parents)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/resourcebuilder.py", line 461, in _make_node
    yield from self._make_callspec(f, name, nsspec, extraargs, parents)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/resourcebuilder.py", line 494, in _make_callspec
    index, _ = gen.send(None)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/resourcebuilder.py", line 412, in _process_requirement
    put_index, val = self.input_parser.resolve_key(name, ns, parents=parents, currspec=nsspec)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/configparser.py", line 430, in resolve_key
    parents=parents, max_index=max_index, write=write)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/configparser.py", line 527, in _resolve_key
    val = f(input_val, **kwargs)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/configparser.py", line 133, in f_
    return f(self, val, *args, **kwargs)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/configparser.py", line 86, in parse_func
    for elem in param]
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/configparser.py", line 86, in <listcomp>
    for elem in param]
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/configparser.py", line 348, in trap_or_f
    return f(self, value, **kwargs)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/configparser.py", line 133, in f_
    return f(self, val, *args, **kwargs)
  File "/Users/CameronVoisey/nnpdfgit/nnpdf/validphys2/src/validphys/config.py", line 73, in parse_func
    return f(self, item, **kwargs)
  File "/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/lib/python3.7/site-packages/reportengine/configparser.py", line 133, in f_
    return f(self, val, *args, **kwargs)
  File "/Users/CameronVoisey/nnpdfgit/nnpdf/validphys2/src/validphys/config.py", line 127, in parse_pdf
    pdf.nnpdf_error
  File "/Users/CameronVoisey/nnpdfgit/nnpdf/validphys2/src/validphys/core.py", line 178, in nnpdf_error
    error = self.ErrorType
  File "/Users/CameronVoisey/nnpdfgit/nnpdf/validphys2/src/validphys/core.py", line 107, in __getattr__
    raise PDFDoesNotExist(self.name) from e
validphys.core.PDFDoesNotExist: LHAPDF

----

[CRITICAL]: A critical error ocurred. This is likely due to one of the following reasons:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In any case, it doesn't really matter. If this was somehow a common problem, we could deal with it by e.g. manually removing it from the list, although that's a bit hacky, or else we can forget about it. Could you please test whether you get the same thing as I do @Zaharid?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well, if you have an incorrect symlink (or anything else) in the PDF directory, that can lead to weird results. The assumption is that the subfolders of that folder are PDF sets and I so don't think the code should try to defend against every possibility.

Copy link
Copy Markdown
Contributor

@siranipour siranipour May 11, 2020

Choose a reason for hiding this comment

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

It's because Cameron has an LHAPDF installation outside his environment,

>>> import lhapdf
>>> lhapdf.paths()
[‘/Users/CameronVoisey/installation/share/‘, ‘/Users/CameronVoisey/miniconda3/envs/nnpdf-dev/share/LHAPDF’]

@voisey
Copy link
Copy Markdown
Contributor

voisey commented May 11, 2020

Regarding 65ad0b0, you also need to update the file name in the index right?

@wilsonmr
Copy link
Copy Markdown
Contributor Author

Regarding 65ad0b0, you also need to update the file name in the index right?

yes 😆

@voisey voisey mentioned this pull request May 11, 2020
@voisey
Copy link
Copy Markdown
Contributor

voisey commented May 11, 2020

Btw, I'm very happy with this script. I find it super useful!

Comment thread validphys2/src/validphys/scripts/vp_list.py
@wilsonmr
Copy link
Copy Markdown
Contributor Author

was discussed at CM yesterday to add at least a simple test to this

@wilsonmr
Copy link
Copy Markdown
Contributor Author

The tests aren't much they just check there is an output where there should be (and in one case check there isn't an output) but I think this is better than nothing. I was thinking of comparing output to http request however as I started trying to write code for that I realised I was just copying the code which the script uses so it seemed a bit pointless testing it against itself.

Provided this passes I'm happy to merge

def test_listdatasets():
"""Checks listing datasets returns output"""
f = io.StringIO()
cmd = ["pdfs"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm guessing this should be cmd = ["datasets"] or something similar, otherwise it's just the same as test_listpdfs

@voisey
Copy link
Copy Markdown
Contributor

voisey commented May 18, 2020

Should we merge the changes in #779 into here so that we can check that it passes?

@wilsonmr
Copy link
Copy Markdown
Contributor Author

I mean we can see the only test which fails is that one: https://travis-ci.com/github/NNPDF/nnpdf/jobs/334643256#L672 I'd rather that PR got merged and this was rebased on master if we really want to see the checks succeed

@voisey
Copy link
Copy Markdown
Contributor

voisey commented May 18, 2020

Okay, I'm happy just to merge this then

@voisey
Copy link
Copy Markdown
Contributor

voisey commented May 18, 2020

It passes now. You happy for the big green button to be pressed @Zaharid?

@Zaharid Zaharid merged commit e588ba4 into master May 18, 2020
@scarlehoff scarlehoff deleted the add-vp-list branch March 5, 2025 15:43
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.

4 participants