Skip to content

Conversation

@max-models
Copy link
Member

@max-models max-models commented Nov 17, 2025

Solves the following issue(s):

This PR deletes 30-40% of Struphy 🔥

Closes #53

Core changes:

  • Remove src/struphy/eigenvalue_solvers/
  • Remove testing of legacy code

@max-models max-models linked an issue Nov 17, 2025 that may be closed by this pull request
@max-models
Copy link
Member Author

What is your opinion on this? Should we remove the legacy code or not? @spossann

@spossann
Copy link
Member

What is your opinion on this? Should we remove the legacy code or not? @spossann

Remove!

@max-models
Copy link
Member Author

What is your opinion on this? Should we remove the legacy code or not? @spossann

Remove!

And for the unit tests comparing results with the legacy code, do we simply remove the comparisons? For example:

assert xp.allclose(val1, val_legacy_1)
assert xp.allclose(val2, val_legacy_2)
assert xp.allclose(val3, val_legacy_3)

@spossann
Copy link
Member

What is your opinion on this? Should we remove the legacy code or not? @spossann

Remove!

And for the unit tests comparing results with the legacy code, do we simply remove the comparisons? For example:

assert xp.allclose(val1, val_legacy_1)
assert xp.allclose(val2, val_legacy_2)
assert xp.allclose(val3, val_legacy_3)

Yes, let's see if we can come up with better tests than comparing to legacy, or if we even can remove some tests. We can have an overview from the diff once legacy (and asserts such as above) are removed.

@max-models
Copy link
Member Author

What is your opinion on this? Should we remove the legacy code or not? @spossann

Remove!

And for the unit tests comparing results with the legacy code, do we simply remove the comparisons? For example:

assert xp.allclose(val1, val_legacy_1)
assert xp.allclose(val2, val_legacy_2)
assert xp.allclose(val3, val_legacy_3)

Yes, let's see if we can come up with better tests than comparing to legacy, or if we even can remove some tests. We can have an overview from the diff once legacy (and asserts such as above) are removed.

Is everything in src/struphy/eigenvalue_solvers legacy?

@spossann
Copy link
Member

What is your opinion on this? Should we remove the legacy code or not? @spossann

Remove!

And for the unit tests comparing results with the legacy code, do we simply remove the comparisons? For example:

assert xp.allclose(val1, val_legacy_1)
assert xp.allclose(val2, val_legacy_2)
assert xp.allclose(val3, val_legacy_3)

Yes, let's see if we can come up with better tests than comparing to legacy, or if we even can remove some tests. We can have an overview from the diff once legacy (and asserts such as above) are removed.

Is everything in src/struphy/eigenvalue_solvers legacy?

yes

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.

Delete all legacy code

3 participants