Improve scale height consistency#145
Conversation
…dernized the gen_point API
…uch even-order noise to be worth keeping
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Add HDF5 file writing to `exp` and HDF file reading to `pyEXP` [WIP]
|
Another fall-back option is to create new C++ version of |
- Replace old 'cylcache' with a version uses the pyEXP constructor - Checks for the Python3 runtime - Disable pybind11 dependence if not found - CMake will not configure if Python3 is not available with pyEXP requested
|
I agree with both of your points:
Notes
|
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 54 out of 55 changed files in this pull request and generated 5 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
utils/PhaseSpace/diffpsp.cc
Outdated
| if (I1min>0) I1min = Emin; | ||
| if (I1max>0) I1max = Emax; |
There was a problem hiding this comment.
Logic error: the conditions should check if the values are negative (not set), not positive. Should be 'if (I1min<0)' and 'if (I1max<0)' to match the initialization values and the comment context.
| if (I1min>0) I1min = Emin; | |
| if (I1max>0) I1max = Emax; | |
| if (I1min<0) I1min = Emin; | |
| if (I1max<0) I1max = Emax; |
There was a problem hiding this comment.
Actually I'm realising this might be wrong, depending on how energy is defined (e.g. positive or negative). Can we fix and also add a comment here about the logic?
There was a problem hiding this comment.
Oh, yeah, that's a mess. I'll comment a proposed fix.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 54 out of 55 changed files in this pull request and generated 6 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
michael-petersen
left a comment
There was a problem hiding this comment.
Lots of changes here; but standard workflows still operate as expected, and this does bring definitions in line with the literature.
No compile problems, will assume we are good to go now.
The Issue
expandpyEXPmake cylindrical bases assuming aThis is confusing. It seems that this latter version is the standard in astronomy. In the short term, I propose adding a new$\mbox{sech}^2(z/2h)$ .
expandpyEXPparameter,sech2which changes the disk conditioning function to useIn the long run, we can deprecate the feature by adding
oldsech2to get the original behavior, followed by dropping the parameter altogether.