Skip to content

Fixing next effective exponents table#731

Merged
voisey merged 3 commits into
masterfrom
fixing_eff_exps
May 1, 2020
Merged

Fixing next effective exponents table#731
voisey merged 3 commits into
masterfrom
fixing_eff_exps

Conversation

@siranipour
Copy link
Copy Markdown
Contributor

Closes #730

@siranipour siranipour requested review from Zaharid and wilsonmr April 21, 2020 10:16
Comment thread validphys2/src/validphys/eff_exponents.py Outdated
Comment thread validphys2/src/validphys/eff_exponents.py Outdated
@wilsonmr
Copy link
Copy Markdown
Contributor

does that look like it's working to you? I think it looks ok for me - basically there was no need to use the internal table since we don't even care abou the previous exponents for this action and I seperated the two tables in #684 and so it just makes sense to use that

@siranipour
Copy link
Copy Markdown
Contributor Author

does that look like it's working to you? I think it looks ok for me - basically there was no need to use the internal table since we don't even care abou the previous exponents for this action and I seperated the two tables in #684 and so it just makes sense to use that

Yep the last commit looks good to me!

@wilsonmr
Copy link
Copy Markdown
Contributor

cool, if you could add a test that'd be good, I'm not sure if the tests already use a fit, I have some low replica dis only closure tests which will produce potentially garbage numbers but will be quick to download 191015-mw-001 is one, might be sufficient to make a table comp to check the values are the same each time etc.

@wilsonmr
Copy link
Copy Markdown
Contributor

or if you know of another low replica fit

@siranipour
Copy link
Copy Markdown
Contributor Author

I've added a quick test to ensure the ite2 runcard predicted by vp matches the one I just generated using 191015-mw-001 as a dummy ite1

@Zaharid
Copy link
Copy Markdown
Contributor

Zaharid commented Apr 29, 2020

Looks fine. But I wonder if we can afford to download two fits in the CI....

@siranipour
Copy link
Copy Markdown
Contributor Author

Yeah I was wondering this, but the 2nd fit is empty in the sense that it doesn't have any replicas in it, so it's as lightweight as possible

Copy link
Copy Markdown
Contributor

@Zaharid Zaharid left a comment

Choose a reason for hiding this comment

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

Looks fine. Thanks!

@voisey voisey merged commit 330a53c into master May 1, 2020
@voisey voisey deleted the fixing_eff_exps branch May 1, 2020 15:35
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.

Next effective expontents has empty lists

4 participants