search

catrionamurray / chromatic_fitting

chromatic_fitting tool to perform multi-wavelength spectrophotometry built on top of chromatic
MIT License
10 stars 1 forks source link

add more flexible operations to make `CombinedModel` #3

Closed zkbt closed 2 years ago

zkbt commented 2 years ago

I tried out a thing on try-combined-model-operations to switch from the user interacting directly with .attach_models to just using mathematical operations (+-*/). It seems to work?!

zkbt commented 2 years ago

Oh, sorry, I have my atom set up to automatically run black on anything I save, so it made a bunch of fussy automatic formatting changes.

catrionamurray commented 2 years ago

Looks good! How does it work if I tried A = B + C - D? Does it iteratively figure out how to combine them?

catrionamurray commented 2 years ago

I also had a thought which is that I liked having the names in the dictionary I passed to CombinedModel so that I could do clever stuff later with plotting where I could separate out the individual components. I don't know whether it's possible with this because you don't supply a name for each model - it gets called 'left' or 'right' if I've understood correctly

zkbt commented 2 years ago

Looks good! How does it work if I tried A = B + C - D? Does it iteratively figure out how to combine them?

It didn't last week, but I think that in principle it could with slightly more careful parameter management. I can take a look!

I also had a thought which is that I liked having the names in the dictionary I passed to CombinedModel so that I could do clever stuff later with plotting where I could separate out the individual components. I don't know whether it's possible with this because you don't supply a name for each model - it gets called 'left' or 'right' if I've understood correctly

That's a really good point that it would be extremely helpful to have names for the model components of a CombinedModel. One other option (which I wound up implementing for Rainbows to be able to compare them with MultiRainbow) would be to give each Model a .name attribute (that could have some reasonable default for each class) that could be used to store these instead?

catrionamurray commented 2 years ago

I've adapted this into the systematics_model branch with some changes to include naming. Not sure what's best to do with this pull request now?