-
Notifications
You must be signed in to change notification settings - Fork 23
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
Issue 41 cimulator buid #47
base: bump-build-version
Are you sure you want to change the base?
Conversation
increased timeout
…chase/QOKit into issue_41_cimulator_buid black .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor comments. Please also note that the workflow should succeed in Github actions (currently failing).
setup.py
Outdated
super().run | ||
except Exception as e: | ||
print("No C/C++ enviroment setup to compile the C simulator. Installing Python Simulator") | ||
if python_only is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this condition be used directly on line 23 below instead of creating an intermediary QOKIT_PYTHON_ONLY
variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need that env variable to control workflow/debug
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed to if python_only is not None:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like more tests are failing now...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like more tests are failing now...
It should right? This is python build only no c-build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did the tests start failing though? Responding in this thread, but not sure which change triggered it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did the tests start failing though? Responding in this thread, but not sure which change triggered it
E AssertionError: assert 'c' in ['python']
E + where ['python'] = get_available_simulator_names('x')
Trigged by env variable setup QOKIT_PYTHON_ONLY=True. Intended behavior for that test.
Why did the tests start failing though? Responding in this thread, but not sure which change triggered it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's intended, it should catch the error, no?
tests/test_simulators_builds.py
Outdated
from qokit.fur import get_available_simulator_names | ||
|
||
# when fail tests only runs in Github actions. Change to true to run locally | ||
IN_GITHUB_ACTIONS = os.getenv("GITHUB_ACTIONS") == "fales" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo! "fales" -> "fails"
tests/test_simulators_builds.py
Outdated
IN_GITHUB_ACTIONS = os.getenv("GITHUB_ACTIONS") == "fales" | ||
|
||
|
||
@pytest.mark.skipif(IN_GITHUB_ACTIONS, reason="Test runs only in Github Actions.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: add graceful handling of this in python pipeline
Status update: there are two main problems |
Update: the Mac build failure is due to clang not supporting openmp. Installing GCC should resolve the issue, but I can't get make to use the right compiler |
fixed following pipeline issues:
|
Add test which check that c simulator is built correctly