-
Notifications
You must be signed in to change notification settings - Fork 129
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
[FTheoryTools] Compute all well-quantized and vertical G4-fluxes #4286
[FTheoryTools] Compute all well-quantized and vertical G4-fluxes #4286
Conversation
20d1445
to
fd4ec6a
Compare
68baa1a
to
936114f
Compare
Ok - this master piece (hopefully?) is finally getting ready. "What is in this PR?" is hear you ask:
I am waiting for the tests to pass now/will make adjustments so that they do. Beyond this, there are the following improvements that are possible:
@apturner @emikelsons maybe we can discuss more shortly. If you find the time, take a (brief) look and let me know your thoughts. |
d36f44d
to
2d85cf3
Compare
This is ready for your reviews @apturner and @emikelsons . |
2d85cf3
to
709229a
Compare
271681c
to
d1a2bf6
Compare
d1a2bf6
to
6f6ee11
Compare
28d81e4
to
16d7dd6
Compare
@emikelsons and I found a bug in this PR yesterday, which we fixed yesterday. I have squashed all of those changes into one (hopefully) working commit. To verify that indeed things are working as intended, the tests have been extended accordingly. Also, I have extended on the documentation. |
8498baf
to
3bcd844
Compare
I am confident that the tests pass once #4341 is resolved. @apturner and @emikelsons , could you please review this PR and tell me if it is good to go as is? Certainly, this PR/change is not perfect. Among others, we could look for better performance. Add an attribute like Still - I have postponed these improvements to a point in the future. |
Maybe we could add these points to our to-do list in #2699. And of course, we should gauge at some point, which of these tasks have been completed/need attention (and if so how urgent) etc... |
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 and this has been extensively tested.
9687b02
to
a3c2829
Compare
a3c2829
to
7f7fd50
Compare
Pull request was closed
Sadly still WIP.cc @apturner @emikelsons