-
Notifications
You must be signed in to change notification settings - Fork 238
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
Q analysis with just part of the modes #47
Conversation
Allow for quantum analysis for only some of the modes which can now be sent as a list
Finally someone uses the develop branch🤩 |
Can you send me the code you used to run this? I'm a bit confused, why you didn't add a |
Probably just a mistake with working with GitHub , I haven't worked with it
in the pass. I'll check my local repository tommarow.
…On Wed, Aug 19, 2020, 8:36 PM Daniel Cohen Hillel ***@***.***> wrote:
Can you send me the code you used to run this? I'm a bit confused, why you
didn't add a modes argument to
analyze_variation (although you did write it in the method doc-string)?
Also, I'm not sure how if it's a good practice to change the attributes
self.n_modes and self.modes temporarily, I'm guessing you did this
because some of the other functions use
these attributes, but it's probably better to pass these as arguments
since it might mess up some other part of the code. I'll look
at it more carefully later 👌 other than that it looks nice 😄
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#47 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AOD23AZIC6MKYRR23J6PHD3SBQESVANCNFSM4QE73ZVQ>
.
|
Great! Let me know if there is any issue. Can probably start testing next week or so in more depth
…Sent from my iPhone
On Aug 19, 2020, at 1:52 PM, asafdi ***@***.***> wrote:
Probably just a mistake with working with GitHub , I haven't worked with it
in the pass. I'll check my local repository tommarow.
On Wed, Aug 19, 2020, 8:36 PM Daniel Cohen Hillel ***@***.***>
wrote:
> Can you send me the code you used to run this? I'm a bit confused, why you
> didn't add a modes argument to
> analyze_variation (although you did write it in the method doc-string)?
> Also, I'm not sure how if it's a good practice to change the attributes
> self.n_modes and self.modes temporarily, I'm guessing you did this
> because some of the other functions use
> these attributes, but it's probably better to pass these as arguments
> since it might mess up some other part of the code. I'll look
> at it more carefully later 👌 other than that it looks nice 😄
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#47 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AOD23AZIC6MKYRR23J6PHD3SBQESVANCNFSM4QE73ZVQ>
> .
>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Fix of Q analysis modes update, print of results is still problamatic
I committed the corrected version and made sure it works on properly in my
local repository. I made a few tests and seems to work properly except for
an issue with the print.
On Wed, Aug 19, 2020 at 9:19 PM Zlatko Minev <notifications@github.com>
wrote:
… Great! Let me know if there is any issue. Can probably start testing next
week or so in more depth
Sent from my iPhone
> On Aug 19, 2020, at 1:52 PM, asafdi ***@***.***> wrote:
>
>
> Probably just a mistake with working with GitHub , I haven't worked with
it
> in the pass. I'll check my local repository tommarow.
>
> On Wed, Aug 19, 2020, 8:36 PM Daniel Cohen Hillel <
***@***.***>
> wrote:
>
> > Can you send me the code you used to run this? I'm a bit confused, why
you
> > didn't add a modes argument to
> > analyze_variation (although you did write it in the method doc-string)?
> > Also, I'm not sure how if it's a good practice to change the attributes
> > self.n_modes and self.modes temporarily, I'm guessing you did this
> > because some of the other functions use
> > these attributes, but it's probably better to pass these as arguments
> > since it might mess up some other part of the code. I'll look
> > at it more carefully later 👌 other than that it looks nice 😄
> >
> > —
> > You are receiving this because you authored the thread.
> > Reply to this email directly, view it on GitHub
> > <#47 (comment)
>,
> > or unsubscribe
> > <
https://github.com/notifications/unsubscribe-auth/AOD23AZIC6MKYRR23J6PHD3SBQESVANCNFSM4QE73ZVQ
>
> > .
> >
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub, or unsubscribe.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#47 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AOD23A7AJEXFZGWBYNV6F2TSBQJTNANCNFSM4QE73ZVQ>
.
|
Seems legit, although I'm still not so sure about changing the attributes. Maybe I'll try to modify it a bit later when I get to the lab:+1: |
I agree, that is why I only changed the attributes temporarily and returned
then to their original values at the end of the analyze variation function.
I am mainly concerned about the printed results, I tried to adjust them to
fit the reduced analysis as best as possible but I might have missed some.
…On Thu, Aug 20, 2020, 4:35 PM Daniel Cohen Hillel ***@***.***> wrote:
Seems legit, although I'm still not so sure about changing the attributes.
Maybe I'll try to modify it a bit later when I get to the lab👍
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#47 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AOD23A4LGXDJXULI3FERLVLSBURCVANCNFSM4QE73ZVQ>
.
|
Allow for quantum analysis for only some of the modes which can now be sent as a list