Skip to content

Python closure sampling#1651

Closed
comane wants to merge 33 commits into
masterfrom
python_closure_sampling
Closed

Python closure sampling#1651
comane wants to merge 33 commits into
masterfrom
python_closure_sampling

Conversation

@comane
Copy link
Copy Markdown
Member

@comane comane commented Dec 15, 2022

Main modification: Level 1 data used for closure tests generated by validphys.pseudodata.make_replica instead of libnnpdf.experiments.MakeClosure.
Closure tests no longer depend on the prepare_nnpdf_rng function which uses the c++ rng (see #1623 ). As a consequence "seed" and "rngalgo" are no longer needed in a closure test runcard, the random initializer only depends on "filterseed"

@comane comane requested review from Zaharid and scarlehoff December 15, 2022 17:04
Comment thread validphys2/src/validphys/commondataparser.py Outdated
Comment thread validphys2/src/validphys/commondataparser.py Outdated
Comment thread validphys2/src/validphys/filters.py Outdated
Comment thread validphys2/src/validphys/commondataparser.py Outdated
Comment thread validphys2/src/validphys/commondataparser.py Outdated
Comment thread validphys2/src/validphys/commondataparser.py Outdated
Comment thread validphys2/src/validphys/tests/test_pseudodata.py Outdated
@Zaharid
Copy link
Copy Markdown
Contributor

Zaharid commented Dec 28, 2022

I think it should be like

def write_commondata_data(commondata, buffer):
    header = f"{commondata.setname} {commondata.nsys} {commondata.ndata}\n"
    buffer.write(header)
    commondata.table.to_csv(buffer, sep="\t", header=None)

Then we may (or not) have a simple function like

def write_commondata_to_file(commondata, path):
    with(open(path, "w")) as f:
        write_commondata_data(commondata, f)

or else have the equivalent functionality in the export function or whatever.

The idea is that one can use the lower level interface to write to things that are not quite files such as memory maps, compressed archives or indeed strings (using stringio as in the example).
This is all a minor point anyway and feel I could have explained better.

Comment thread validphys2/src/validphys/filters.py Outdated
Comment thread validphys2/src/validphys/filters.py Outdated
Comment thread validphys2/src/validphys/tests/test_pseudodata.py Outdated
Comment thread validphys2/src/validphys/pseudodata.py Outdated
Comment thread validphys2/src/validphys/pseudodata.py Outdated
level1_data = make_replica(level0_commondata_wc, filterseed, covmat,
sep_mult=False, genrep=True)

group_index = groups_index([data]) # already set cuts
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.

Why not having group_index as a provider here instead?

Copy link
Copy Markdown
Member Author

@comane comane Jan 4, 2023

Choose a reason for hiding this comment

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

I think there is already one called experiments_index in results.py.
In the new commits I pass it to the filter_closure_data_by_experiment function which loops over the experiments so that the right indexes are passed to the _filter_closure_data function

Comment thread validphys2/src/validphys/pseudodata.py Outdated



def make_level1_data(data,commondata_wc,level0_commondata_wc,filterseed):
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 think we should have a vp provider that does commondata_wc provider, so that this function can be used as an action.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added a commondata_wc function in results.py. Also, in the new commits the make_level0_data has been renamed level0_commondata_wc

@Zaharid
Copy link
Copy Markdown
Contributor

Zaharid commented Feb 8, 2023

This now lives in #1660.

@scarlehoff scarlehoff deleted the python_closure_sampling branch March 5, 2025 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants