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

consider merging 1 and 2 group netsim modules #805

Open
chad-klumb opened this issue Jan 24, 2023 · 2 comments
Open

consider merging 1 and 2 group netsim modules #805

chad-klumb opened this issue Jan 24, 2023 · 2 comments

Comments

@chad-klumb
Copy link
Collaborator

This could save some code now, and possibly set up stratification on > 2 groups down the road.

chad-klumb added a commit that referenced this issue Mar 7, 2023
chad-klumb added a commit that referenced this issue Mar 7, 2023
chad-klumb added a commit that referenced this issue Mar 8, 2023
chad-klumb added a commit that referenced this issue Mar 8, 2023
@smjenness
Copy link
Collaborator

Although this code is inefficient from a machine perspective, it is designed this way so that is easier to read from a human perspective (specifically beginning epimodel users). The idea is that users that are unconcerned with multi-group models can dive into the 1 group code and use that as a template more easily than if they had to pull apart a more complex function that used a lot of conditional logic.

That's not to say that all of this is designed as clean and elegantly as it could. If you see room for improvement, go for it. But the underlying architecture of using multiple functions that have some redundancy in them should remain. Thanks!

@chad-klumb
Copy link
Collaborator Author

Okay. I had already pushed some related changes to the refactor_groups branch. I haven't polished them or given them final review, but they convey the basic idea I had in mind for this issue. Changes were made in such a way as to avoid deepening the conditional logic.

I will abstain from further work on this issue unless there's a specific endpoint you'd like me to work towards.

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

No branches or pull requests

2 participants