search

Chris-Pedersen / LaCE_manager

Cosmological emulator for the 1D flux power spectrum of the Lyman-alpha forest
3 stars 1 forks source link

Split emulator into submodule #57

Closed Chris-Pedersen closed 2 years ago

Chris-Pedersen commented 2 years ago

I think we are ready to do this now. I will rename this package to lace_manager, and all emulator functionality will be imported from igmhub/lace as a submodule. So in this issue, I will refer to this repo as lace_manager, and the igmhub repo as lace. Two initial things to decide are:

  1. Do we want to keep the cosmo modules imported from lace? They will be utilised in both repos, but the intention is for lace to be standalone, whereas lace_manager will always be run with lace as a submodule.
  2. We have a load of other modules in lace_manager.emulator that are currently not in lace , such as the simplest_emulator, z_emulator etc. These are left over from all the architectures we played around with back in 2020 - shall we just delete these, or move them to lace? I guess it's possible once someone starts working with P3D or the bispectrum that they might want to explore these things again to re-evaluate performance. There are also a large number of testing flags for the lace.emulator.gp_emulator that we can think about removing to make the code less bloated.

@andreufont interested to hear your thoughts!

andreufont commented 2 years ago

@Chris-Pedersen - I'd be happy to get rid of most of the old code, since we can always go back to the tagged version if we ever wanted to look at these. We even have the LyaCosmoParams repo (that was the one used in the 2021 paper, right?). If there was an emulator that you imagine maybe being useful in the future, we could also have an lace.extra_emulators folder with those that is only in lace_manager, not in lace. However, it might be the case that none of these will be useful any time soon. I'd just do whatever is simpler for you, honestly.

In terms of the cosmo module, if possible it would be great if it was just in the standalone lace repo. Is there any reason not to?

Chris-Pedersen commented 2 years ago

Sounds good, will trim the excessive code!

Is there any reason not to?

Not really, the only thing I could think of is that at some point we might have to make tweaks there that are not used in lace but only in lace_manager, but that's fine I guess.

andreufont commented 2 years ago

I know what you mean, but I think that once the compression project is done, it is possible that future projects will not use lace_manager but some new Cobaya module... So I would try to do the simplest thing here.

andreufont commented 2 years ago

The project on "P1D forecast" will probably use lace_manager, but that one does not require new changes in the cosmo object, and if it did, we might as well put it in lace

Chris-Pedersen commented 2 years ago

Sounds good, even if there are some changes required in a submodule I think it's preferable to having duplicate code.

Chris-Pedersen commented 2 years ago

This is done now in #62 . Might be a few bumps with certain notebooks but have updated the key ones. Installation procedure is a bit clunky for now but we don't expect general users to use lace_manager so I think it's ok to leave it like this.

Chris-Pedersen commented 2 years ago

Oops I actually made a mistake here in thinking we don't need separate environment variables for lace and lace_manager. Will fix this tomorrow.

Chris-Pedersen commented 2 years ago

Fixed and working now