SWIFTSIM / emulator

The SWIFT simulation cosmological emulator (swift-emulator)
GNU Lesser General Public License v3.0
5 stars 2 forks source link

Add polynomial surface fit model #4

Closed Moyoxkit closed 3 years ago

Moyoxkit commented 3 years ago

Bit surprised myself as well, but i think this is all that is needed to fit polynomial models. We will need some better sampled data to see if it actually helps

Moyoxkit commented 3 years ago

What do you think of this? Allowing the users to supply a string with the fit_model name instead, this way we can also check whether they know what they are doing. Should probably rename the variable to mean_model in this case

JBorrow commented 3 years ago

I was thinking more that we separate out the mean models (we're no doubt going to have more in the future) and have people pass in the mean model as its own object... I can take a look at doing this, if you don't feel comfortable (after we've merged this version). If you think you could give that a crack give it a go though.

Moyoxkit commented 3 years ago

Yeah that is the other option. The thing I like about the current setup though is that it's very easy to try a couple models without knowing much about the technical details of putting it all in a format george likes. What we could do is create an additional option mean_model = "custom", which allows you to put it in by hand.

JBorrow commented 3 years ago

Ah but we'd provide these - passing things as strings and relying that they end up corresponding to the correct thing is dangerous. I was thinking:

from swiftemulator.mean_models import Polynomial

...
my_gpe.fit_model(
  ...,
  mean_model=Polynomial
)
JBorrow commented 3 years ago

This also has the benefit that it makes the code much cleaner, and if we provide other emulators (using different styles of kernels, or something else entirely) we can use this same protocol (much in the same way as george does).

Moyoxkit commented 3 years ago

Yes that would be nicer indeed! Then we could also configure the mean model separately. For example: from swiftemulator.mean_models import Polynomial ` ... my_gpe.fit_model( ..., mean_model=Polynomial(degree=2) )`

JBorrow commented 3 years ago

Exactly, good idea with the configuration options.

JBorrow commented 3 years ago

So are we happy to add this as is then we can retrofit a better mean model situation later, or are you working on that?

Moyoxkit commented 3 years ago

Yes, let's add this like this and add that later. I think it's better to focus on some other things for now, besides we only have two options at the moment so it's not that messy (yet).