Skip to content
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

rough implementation of the mapmaker pipeline tools. Includes mappin… #321

Merged
merged 5 commits into from
Mar 20, 2020

Conversation

keskitalo
Copy link
Member

…g a number of madam parameters to shared arguments in a backwards-compatible way.

… a number of madam parameters to shared arguments in a backwards-compatible way.
Copy link
Member

@tskisner tskisner left a comment

Choose a reason for hiding this comment

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

The mapmaker changes look fine. We just need to revert the formatting of external sources.

src/toast/pybind11/pybind11/__init__.py Outdated Show resolved Hide resolved
src/toast/pybind11/pybind11/__main__.py Outdated Show resolved Hide resolved
@tskisner
Copy link
Member

Ok, this looks good, will merge after tests pass.

@tskisner
Copy link
Member

Looks like our extensive map-making unit tests now exceed the travis allowed run time. We'll need to figure out a way of making these cheaper.

@ickc
Copy link
Contributor

ickc commented Feb 22, 2020

I think the time limit is about 50 min. if that hasn't changed.

The recommended way to do this is to further split it into a matrix. Since TOAST test framework already has a way to select a subset of tests, it can be split into smaller granularity. This is useful to accelerate the turn-around time of CI too, provided that the parallelism is not too much to exhaust the no. of parallel builds available. (3 parallel builds for open source for free.)

@ickc
Copy link
Contributor

ickc commented Feb 22, 2020

Is it correct to say that the name passes to toast test are those in https://github.com/hpc4cmb/toast/tree/master/src/toast/tests?

(quick fix for now) Is it currently possible to ignore a test as well? e.g. One CI do ops_mapmaker and another do all but ops_mapmaker.

(long term) I suppose the toast test arg can be a list of those names? One option is to create 3 instances, manually splitting those names in 3 groups and hard-code it in .travis.yml. But I wonder that would be too manual hence someone adding a new test might forgot to add it in .travis.yml.

@tskisner
Copy link
Member

As you know, our travis tests already have a job matrix with 3 entries, testing different compiler versions. For each job entry we run both the serial and MPI tests, so that is a natural place to split. This would result in 6 jobs, though only 3 would run at once.

I'll go make that change, but in the long term we should also consider shrinking the size of the test cases to make them more practical to run.

@tskisner
Copy link
Member

The unit tests pass now. In one case, PySM failed to download a data file. We are going to be bitten by this problem over and over. @zonca, is there any way to pre-cache the PySM data files? Perhaps a script to manually fetch them all into the package data directory? Or an import statement that will be guaranteed to get everything?

This way I can do that when building the travis dependency tarball (and also when installing at NERSC).

@zonca
Copy link
Member

zonca commented Mar 19, 2020

@tskisner no, and you definitely don't want to cache all the input data but only the ones you actually need for a specific run.
The easiest would be to simplify the sky model so that maybe only 1 component needs to be downloaded.
It would be nice to have a basic set of input components which don't require any input file, like random gaussian foregrounds generated on the fly, but won't have time to work on that anytime soon.

@tskisner
Copy link
Member

Merging this now, although we do need to find a better solution. One possibility is when building the travis dependency tarballs to separately install toast and run the tests inside the docker container, in order to trigger the caching before making the tarballs.

@tskisner tskisner merged commit 0300a4e into master Mar 20, 2020
@tskisner tskisner deleted the mapmaker_pipeline_tools branch March 20, 2020 20:47
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.

4 participants