Skip to content

Remove CBrock, CBrockDisk, Hernquist#74

Merged
michael-petersen merged 1 commit intomainfrom
SimplifyBases
May 12, 2024
Merged

Remove CBrock, CBrockDisk, Hernquist#74
michael-petersen merged 1 commit intomainfrom
SimplifyBases

Conversation

@michael-petersen
Copy link
Copy Markdown
Member

This branch removes the bases that a superseded by more adaptive versions. For the Clutton-Brock (1973) and Hernquist & Ostriker (1992) basis, this is sphereSL; for the Clutton-Brock (1972) disk basis, this is flatdisk.

@The9Cat
Copy link
Copy Markdown
Member

The9Cat commented May 9, 2024

This looks fine to me. And I've checked that it compiles without incident. If you have nothing else to add here, let's merge it.

@michael-petersen
Copy link
Copy Markdown
Member Author

I'm thinking we want to keep bessel, as a unique basis? That was my only open question.

@The9Cat
Copy link
Copy Markdown
Member

The9Cat commented May 9, 2024

It might be worth keeping bessel because it provides something that the others do not. Athough it does not have pyEXP integration or cuda support. Neither of those would be all that hard to do. We might want to try it to make sure it works correctly...

@michael-petersen
Copy link
Copy Markdown
Member Author

Right, I agree if we keep bessel (probably the right call) we should have some sort of test. Any thoughts on what this would be?

pyEXP integration is a bonus, but could also be a minor rev later if requested.

@The9Cat
Copy link
Copy Markdown
Member

The9Cat commented May 10, 2024

For a test, perhaps:

  • Realize a King model and make sure it's stays in equilibrium
  • Use Oskipkov to make a radial orbit instability

Yes, adding to pyEXP would be an hour of work. Not a big deal. CUDA would be similar. Part of the work is already done because the bessel implementation uses tables, and that's the same as Sphere, so it can be leveraged. Thinking about that, the right thing to do is to move the CUDA Sphere implementation to SphericalBasis and use the same code for both.

@michael-petersen
Copy link
Copy Markdown
Member Author

As the bessel fixes are being implemented in #73, I'll merge this in now.

@michael-petersen michael-petersen merged commit bcc60aa into main May 12, 2024
@michael-petersen michael-petersen deleted the SimplifyBases branch May 29, 2024 15:37
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