Skip to content
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

Duplicate instances of self._setup_complex_phase() in relative_setup.py #1219

Open
zhang-ivy opened this issue Jul 28, 2023 · 2 comments · Fixed by #1233
Open

Duplicate instances of self._setup_complex_phase() in relative_setup.py #1219

zhang-ivy opened this issue Jul 28, 2023 · 2 comments · Fixed by #1233
Assignees
Labels
priority: low low priority

Comments

@zhang-ivy
Copy link
Contributor

zhang-ivy commented Jul 28, 2023

I noticed that self._setup_complex_phase() is called twice.

@zhang-ivy
Copy link
Contributor Author

Iván notes that this is not super high priority since we are in the midst of overhauling the API anyway

@ijpulidos
Copy link
Contributor

Yes,the comment on

# We must generate the complex topology and positions before the SystemGenerator is created because if the
explains why we needed that at that instance but, as discussed, we can quickly investigate what happens if we remove the second call, since that seems redundant and it shouldn't be doing anything new.

As a reference, when I worked in the refactor for the NEQ cycling protocol, the RelativeFEPSetup.__init__ method of is more organized and it's only setting up the phases at the end of it, as per

self._setup_phases(
(just to keep in mind).

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

Successfully merging a pull request may close this issue.

2 participants