Skip to content

Conversation

Nomos11
Copy link
Collaborator

@Nomos11 Nomos11 commented Jul 30, 2025

No description provided.

Copy link

github-actions bot commented Jul 30, 2025

Test Results

    6 files      6 suites   7m 21s ⏱️
1 219 tests 1 157 ✅  62 💤 0 ❌
7 314 runs  6 942 ✅ 372 💤 0 ❌

Results for commit f903de7.

♻️ This comment has been updated with latest results.

@Nomos11 Nomos11 requested a review from shumpohl August 1, 2025 08:58
@Nomos11 Nomos11 marked this pull request as ready for review August 19, 2025 13:59
@Nomos11
Copy link
Collaborator Author

Nomos11 commented Aug 19, 2025

If #920 and this are marged, one can start testing the changes outsourced to qupulse_hdawg to converge to a working main branch covering the features required for usage at the experiments / with elicit

@shumpohl
Copy link
Member

I do not understand the purpose of this hack. I remember we discussed it already somewhere...

A proper hack would be to do

def get_caller_self():
        frame = sys._getframe(2)
        try:
            return frame.f_locals.get('self')
        finally:
            # LLM suggested that to avoid reference cycles. Unsure if necessary. I do not think so
            del frame

class A:
    def call_stuff(self):
        stuff()

def stuff():
    print('called by', get_caller_self())

@Nomos11
Copy link
Collaborator Author

Nomos11 commented Aug 22, 2025

def get_caller_self():
        frame = sys._getframe(2)
        try:
            return frame.f_locals.get('self')
        finally:
            # LLM suggested that to avoid reference cycles. Unsure if necessary. I do not think so
            del frame

class A:
    def call_stuff(self):
        stuff()

def stuff():
    print('called by', get_caller_self())

the LLM advises against sys._getframe for being a private CPython implementation detail

@shumpohl
Copy link
Member

shumpohl commented Aug 22, 2025

Therefore, it is a proper hack. Yet the better way is to use inspect

import inspect

def get_caller_self():
        inspect.currentframe().f_back.f_back.f_locals.get('self')

class A:
    def call_stuff(self):
        stuff()

def stuff():
    print('called by', get_caller_self())

@Nomos11
Copy link
Collaborator Author

Nomos11 commented Aug 22, 2025

Touché.

i‘ve called a friend of mine who‘s an expert on hacks and he said:
Short answer: nope—this isn’t better. It’s the same fragile trick, just via inspect. inspect.currentframe() is a CPython detail; on other interpreters it may return None. Either way, this is non-portable program logic.

@shumpohl
Copy link
Member

Luckily, nobody uses pypy

@Nomos11
Copy link
Collaborator Author

Nomos11 commented Aug 22, 2025

if you say nicer code syntax > robustness i'll move it to qupulse-hdawg via the hack

@shumpohl
Copy link
Member

Its less about nicer syntax and more about trying to keep the abstraction layer clean to make code easier to reason about.

@Nomos11
Copy link
Collaborator Author

Nomos11 commented Aug 22, 2025

I get & agree that in idealistic scenarios it would be nicer to keep every concept nicely separated, but the reason that so far it was done this way is that otherwise "a lot" of metadata-clutter about the nature of the loop that is to be built a program of, e.g. its inner variables, identifier, etc., would need to be provided to the with_iteration function explicitly (or in some form of metadata-wrapper) to allow the builder to react to different scenarios if the _internal_create_program is not to be cluttered with specifically taylored syntax, and from a code-diff perspective it's therefore a much smaller change to explicitly provide this back-reference for such situations if not relying on hacky and more obfuscated implicit methods to get this.
Since the with_iteration method is strongly tied to the LoopPT anyways and not used anywhere else, the implicit tie between both already exists, or am i missing something conceptually?
How would you go about providing the necessary information to the builder? The other option I see is to create such a metadata-object which collects data form the instance of LoopPT inside the LoopPT and gives it to the builder as an argument of the function, but that would basically be very similar to now just with extra steps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants