Skip to content

mnms runs with mbatch #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

mnms runs with mbatch #8

wants to merge 4 commits into from

Conversation

iabrilcabezas
Copy link
Collaborator

need to run noise_gen and noise_sim.py scripts. I am using mbatch.

mbatch requires --output-dir argument (where .sh and output messages of run are store). also need to transform the args.qid argument because it is read in as a string. I believe these changes should not alter current pipelines that run mnms/scripts/

Copy link
Collaborator

@zatkins2 zatkins2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Irene, could you add your --output-dir argument to noise_all.py as well?

Can you elaborate more on the change to args.qid? The way it was before I think does the same thing: the options supplied to parser.add_argument causes args.qid to be a list of strings

@iabrilcabezas
Copy link
Collaborator Author

it's related to this problem:
Captura de pantalla 2024-10-07 a las 9 28 35

You can reproduce this error with a small script test_mbatch.py like this one:

from mnms import noise_models as nm
import argparse

parser = argparse.ArgumentParser(formatter_class=argparse.ArgumentDefaultsHelpFormatter)

parser.add_argument("--output-dir", dest="output_dir", type=str, default=None)

parser.add_argument('--config-name', dest='config_name', type=str, required=True,
                    help='Name of model config file from which to load parameters')

parser.add_argument('--noise-model-name', dest='noise_model_name', type=str, required=True,
                    help='Name of model within config file from which to load parameters')

parser.add_argument('--qid', dest='qid', nargs='+', type=str, required=True,
                    help='list of soapack array "qids"')

args = parser.parse_args()
print(args.qid)
# args.qid = [item for sublist in args.qid for item in sublist.split()]

model = nm.BaseNoiseModel.from_config(args.config_name, args.noise_model_name,
                                      *args.qid)
print('success')

To run it, we use a test mbatch config file looks like this

root_dir: /home/ia404/gitreps/mnms/scripts/

stages:

  stage_test:
    exec: python
    options:
      config-name: 'act_dr6v4_day'
      noise-model-name: 'tile_cmbmask_daywide'
      qid:
       - 'pa5a_dw'
       - 'pa6a_dw'

    script: scripts/test_mbatch.py

This generates
python scripts/test_mbatch.py --config-name=act_dr6v4_day --noise-model-name=tile_cmbmask_daywide --qid="['pa5a_dw', 'pa6a_dw']" --output-dir /home/ia404/gitreps/mnms/scripts/test/stage_test, which doesn't work because qid ends up being ["['pa5a_dw', 'pa6a_dw']"], pa5a and pa6a all together.

Another alternative:

root_dir: /home/ia404/gitreps/mnms/scripts/

stages:

  stage_test:
    exec: python
    options:
      config-name: 'act_dr6v4_day'
      noise-model-name: 'tile_cmbmask_daywide'
      qid: 'pa5a_dw pa6a_dw'

    script: scripts/test_mbatch.py

But in this case we have
python scripts/test_mbatch.py --config-name=act_dr6v4_day --noise-model-name=tile_cmbmask_daywide --qid='pa5a_dw pa6a_dw' --output-dir /home/ia404/gitreps/mnms/scripts/test/stage_test

which also doesn't work, ['pa5a_dw pa6a_dw'] not separated again.

Adding
args.qid = [item for sublist in args.qid for item in sublist.split()] is the best solution I could find for it to be able to run with mbatch, while also being compatible with older scripts.

@zatkins2
Copy link
Collaborator

zatkins2 commented Oct 7, 2024

Ok, this is very clear. I still wonder if there is a cleaner solution. Can you test what happens in mbatch if you supply qid more than once in a config file?

qid: 'pa5a_dw'
qid: 'pa6a_dw'

@zatkins2
Copy link
Collaborator

zatkins2 commented Oct 7, 2024

Please also target develop :)

@iabrilcabezas iabrilcabezas changed the base branch from main to develop October 7, 2024 20:13
@iabrilcabezas
Copy link
Collaborator Author

It's now targeting develop, and I opened an issue on mbatch, hopefully good to merge now --

@zatkins2
Copy link
Collaborator

zatkins2 commented Oct 9, 2024

As discussed offline, we will hold off on merging until simonsobs/mbatch#25 is fixed on the mbatch side.

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