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

Discussion: getallfactors + getallsummands #69

Open
emerali opened this issue Oct 16, 2020 · 2 comments
Open

Discussion: getallfactors + getallsummands #69

emerali opened this issue Oct 16, 2020 · 2 comments
Assignees

Comments

@emerali
Copy link
Contributor

emerali commented Oct 16, 2020

@elisno brought up the question of whether we should give some sort of warning when calling getallfactors on KroneckerSum or getallsummands on AbstractKroneckerProduct as their behavior may be unexpected when dealing with mixtures of KroneckerProducts and KroneckerSums.

See the short discussion here: #66 (comment)

@MichielStock
Copy link
Owner

I think indeed a warning would be best. Though I would kind of see functions as getallsummands as low-level stuff needed internally but not really accessible to the user? Once you create a Kronecker product, it might not make much sense to pry out the individual matrices again?

P.S. Sorry for my very late response...

@MichielStock MichielStock self-assigned this Nov 10, 2020
@emerali
Copy link
Contributor Author

emerali commented Nov 10, 2020

IMO it shouldn't warn because it's an internal method. It would be kind of annoying for the user to have to deal with these warnings every time the functions are called internally.

However, I think we could circumvent these issues (if we do decide to make the methods public) by having two functions getallsummands and _getallsummands, where the underscore function is used internally and doesn't ever throw warnings, while the other is user-facing and does warn. Not sure if this is the cleanest solution though.

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