-
Notifications
You must be signed in to change notification settings - Fork 19
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
Toast mlmapmaker updates #1053
Toast mlmapmaker updates #1053
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of comments for consideration.
Thanks for refactoring the obs wrapping, which will be useful for the next steps of wrapping the sotodlib binner.
sotodlib/toast/ops/mlmapmaker.py
Outdated
maxiter = Int(500, help="Maximum number of CG iterations") | ||
maxiter = List( | ||
[500], | ||
help="Maximum number of CG iterations. Use commas to control each pass separately.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe change the help string to explicitly say, "List of maximum number of CG iterations for each pass".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Changed.
tests/test_mapmaker_pointing.py
Outdated
try: | ||
from . import _helpers as helpers | ||
except ImportError: | ||
import _helpers as helpers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would the relative import fail? We want to make sure we are always importing the _helpers
module located in the tests
directory, not in other locations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this modification, I can execute this one unit test individually with python tests/test_mapmaker_pointing.py
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of just doing
python -m unittest tests/test_mapmaker_pointing.py
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reverted
Address review feedback.
Undo recent change. Use 'python -m unittest' instead.
Upgrade the TOAST wrapper around the ML mapmaker to support multipass mapmaking and downsampling. Also, add checkpointing capability.
This PR also includes minor tweaks from the MSS3_tweaks branch.