Skip to content

Update mc_gen_checks module for C++ rep gen to data keyword#1087

Merged
siranipour merged 14 commits into
masterfrom
oldcoderepgen
Mar 18, 2021
Merged

Update mc_gen_checks module for C++ rep gen to data keyword#1087
siranipour merged 14 commits into
masterfrom
oldcoderepgen

Conversation

@RosalynLP
Copy link
Copy Markdown
Contributor

I realise this is semi redundant given #866 but I was using this module to generate C++ replicas to compare to the python ones and had to update it to account for the data keyword changes. If people think this is worth merging we can do that.

@RosalynLP
Copy link
Copy Markdown
Contributor Author

NB I've checked that the example runcard mc_gen_checks_example.yaml all works fine

@Zaharid
Copy link
Copy Markdown
Contributor

Zaharid commented Feb 9, 2021

Looks good!

@Zaharid Zaharid requested a review from siranipour February 9, 2021 15:31
Comment thread validphys2/src/validphys/mc_gen_checks.py Outdated
Comment thread validphys2/src/validphys/mc_gen_checks.py Outdated
Comment thread validphys2/src/validphys/mc_gen_checks.py Outdated
Comment thread validphys2/src/validphys/mc_gen_checks.py
RosalynLP and others added 3 commits February 9, 2021 16:52
Co-authored-by: siranipour <43517072+siranipour@users.noreply.github.com>
@siranipour siranipour mentioned this pull request Feb 15, 2021
@RosalynLP
Copy link
Copy Markdown
Contributor Author

I'm not sure exactly what tests would be good for this module... because the actions deal with random numbers then you can't really check the output is unchanged. Could someone please help me with suggesting what is suitable?

@siranipour
Copy link
Copy Markdown
Contributor

Would a regression test like in 6a12bd6 make sense? Just generate some pseudo data, save to disk and make sure we generate the same pseudo data if we seed appropriately until the end of time?

@RosalynLP
Copy link
Copy Markdown
Contributor Author

Would a regression test like in 6a12bd6 make sense? Just generate some pseudo data, save to disk and make sure we generate the same pseudo data if we seed appropriately until the end of time?

ok I see, so a seed specific test

@siranipour
Copy link
Copy Markdown
Contributor

Why is the test using the parquet pull request from report engine @Zaharid

@siranipour
Copy link
Copy Markdown
Contributor

Btw could we call this file mc_gen.py?

@RosalynLP
Copy link
Copy Markdown
Contributor Author

Btw could we call this file mc_gen.py?

Yeah no idea why it's called what it is!

@RosalynLP
Copy link
Copy Markdown
Contributor Author

@siranipour lol thanks XD

@RosalynLP
Copy link
Copy Markdown
Contributor Author

@Zaharid the build has errored and I can't restart it - is this something to do with the changes in reportengine or is it unrelated?

@Zaharid
Copy link
Copy Markdown
Contributor

Zaharid commented Mar 10, 2021

It looks reportengine related. Should be fixed now.

@siranipour
Copy link
Copy Markdown
Contributor

I still can't get the job to restart. This must be a travis problem? We could try force pushing

Comment thread validphys2/src/validphys/tests/test_mcgenchecks.py Outdated
@siranipour siranipour merged commit 2314596 into master Mar 18, 2021
@siranipour siranipour deleted the oldcoderepgen branch March 18, 2021 11:34
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.

3 participants