-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bundled changes: support Windows, add findEndOfInflation() function, add common-subexpression elimination #8
Open
ds283
wants to merge
20
commits into
jronayne:master
Choose a base branch
from
ds283:sampler-version
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- enables installation on Windows, which uses a different path separator (‘\’ rather than ‘/‘). This can be handled automatically by use of os.path.join() rather than hardcoding the separator ‘/‘ - move from distutils to setuptools in moduleSetup.py. Under Windows, the installer has to find the Visual C++ compiler in order to build the C++ part of any Python module. Although distutils is still ‘preferred’, it cannot locate the Visual C++ compiler. However, setuptools will do this automatically (eg. see https://www.microsoft.com/en-gb/download/details.aspx?id=44266), but distutils does not know how to do so and fails - when attempting to add the PyTransport library search paths, no longer assume the directory layout produced by distutils; instead, reflect PEP-370 (https://www.python.org/dev/peps/pep-0370). (Under Windows the layout seen in the field does not quite match that prescribed by PEP-370, which may require further investigation)
- installation occurs into the directory PyTransport/PyTrans, but the paths were previously set up as if it occurred directly into PyTransport
- there’s no risk of duplicating paths using site.addsitedir(), so instead of having multiple copies of the same logic we should re-use the implementation in pathSet()
- accepts Python arrays corresponding to initial conditions, parameters, tolerances, and two doubles specifying the start and end of the search window (Nstop) - integrates the background until the end of the search window is encountered, or epsilon becomes unity - returns either N = Nstop, meaning that the end of inflation was not found, or the time at which epsilon exceeds unity. In future, we could consider returning None if the end of inflation was not found successfully
…found - this seems preferable for idiomatic Python
- final argument now becomes optional and gives size of the search window in e-folds, rather than specifying the absolute position of the end of the window - size of search window defaults to 10,000 e-folds if no value is given
- although nullptr is preferable, it is not supported by GCC 4.8 which is shipped with (eg.) Scientific Linux 7 — still a current-generation product
# Conflicts: # PyTransport/PyTransSetup.py
- for very long potentials, such as the conifold potential, the generated C++ file is too large and cannot be handled by the compiler. To bring the file size down we need to make use of SymPy’s built-in common-subexpression elimination routines to abstract expressions that are multiply-reused. This adds some extra runtime when generating a PyTransport module - add optional status updates so that it is possible to track where PyTransport is in a long calculation that might involve several instances of CSE - switch setuptools compiler flags to C++11 to allow use of ‘auto’ keyword for CSE temporaries without warnings - switch SymPy printing to C++ rather than C so that we pick up the correct standard library routines (ie. with std:: namespace prefixes) - abstract some commonly-used routines such as index replacement for SymPy indexed quantities
- threrefore we can’t apply CSE to it. Instead we have to SymPyfy it before we attempt to do so
…into sampler-version
…into sampler-version
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Roughly, the breakdown of changes incorporated here is:
changes in
PyTransScripts.py
,moduleSetup.py
are just changes to the format of messages printed by each functionchanges in
PyTransSetup.py
are a mix of changes needed to work on Windows (mostly higher up the file, beforedef potential()
), and changes to support use of CSE (indef potential()
anddef field metric()
)PyTrans.cpp has a new function
findEndOfInflation()
which is duplicated from a previous pull request