swagnercarena / paltas

Conduct simulation-based inference on strong gravitational lensing systems.
MIT License
13 stars 12 forks source link

Common base class for extensibility and reduced boilerplate #40

Open JelleAalbers opened 1 year ago

JelleAalbers commented 1 year ago

This introduces two changes:

I put the new base class in a top-level core.py. If you like that, maybe we can move the ConfigHandler stuff there too. We could also squirrel away the base class in an obscure utils file.

One remaining eyesore is the lens light handling. Perhaps we could implement lens light by adding a source model at the lens redshift, instead of collecting separate buckets of lenstronomy arguments? This would require more changes in the test suite though.

The tests will fail until https://github.com/swagnercarena/paltas/pull/38 (or at least https://github.com/swagnercarena/paltas/commit/94bfd3d83c8d3bf650dc53ffbcf093224edbcdd2) is merged and we rebase.

JelleAalbers commented 1 year ago

I experimented with a wrapper to convert Sources to lens light classes here: https://github.com/JelleAalbers/paltas/commit/9d1cee2102e73edbb156e7d3f5a4dd2d9c411583 (+ a few bugfixes in later commits). Let me know if you'd like something like that here (or in a future PR), I'm on the fence as to whether it actually made things clearer or not.

swagnercarena commented 10 months ago

Hi Jelle, what is the status of this pull request? Are you waiting for another review on the changes?