Implement a pointing generator #48
Conversation
|
@mreineck , I am having problems compiling If you tried to set up a Travis CI job for the library, can you share your scripts so that I can integrate your solution here? At the moment the PR does not use |
|
Are you actually using Any I have set up CI with Gitlab, will attach the relevant files in a minute! |
|
No need to upload anything ... the files are |
I have just asked TravisCI to install XCode and use whatever is available; I think that this makes the VM defaulting to
It sounds troublesome at best… I found nothing else than a Reddit topic about this: https://www.reddit.com/r/cpp_questions/comments/9la2e7/technical_reasons_for_apple_restricting_any_and/ |
|
Ok, this build failed too: https://travis-ci.com/github/litebird/litebird_sim/jobs/367303706 It seems you're right: even if the SDK is the most recent one, it fails to build because the OS does not support some functionalities required by the C++ library, as this error message suggest: (So Mac OS X 10.9 doesn't provide functions to allocate a block of aligned memory… really?!?) However, this is in conflict with the TravisCI documentation (https://docs.travis-ci.com/user/reference/osx#macos-version), which states that specifying I think we need some help from people using the Mac. @dpole , might you please try to download the |
I'm pretty sure it provides the function, but htis message is about the In any case, I cannot imagine that this kind of trickery is going on when |
|
BTW, I'm sorry that this code is proving such a hassle to compile... after your success on Windows, I definitely didn't expect this. I'm also building the library regularly with |
Yeah, I believe so, this would explain everything. What puzzles me is the fact that Travis should be using Mac OS X 10.15, not Mac OS X 10.9.
Don't worry, this is the very first C++ library we're integrating in the code, so it makes sense that we're experiencing many problems. Once we're able to understand how to make it work, I expect that other C++ libraries will be far simpler to use. |
I wonder if this is related to the fact that my |
|
Yes! I think that did the trick :) |
You're my hero, thanks a lot! |
|
I still don't feel particularly good about requiring such a recent OS version... I mean, recent compiler versions are easier to install than newer OSs. What do you think, should I try to avoid the |
Mmm, this depends on how often Apple users upgrade their OS. I guess they do it promptly, but it's probably better to ask to other members of the ST that are Apple users if this is true (I know of @NicolettaK , @paganol , @dpole , but there are surely many others). This problem is going to be less and less relevant as time passes, considering that Apple has just announced the successor to 10.15, and that Apple is famous for quickly dropping support for older OSes. (Wikipedia extrapolates old data to guess that 10.14 will be supported till September 2021, i.e., one year from now.) I would go for dropping support for Mac OS X 10.14 and earlier versions, but let's hear for other opinions. |
|
Ehm, I've just checked and I still have Mac OS X 10.14... |
Argh, then perhaps you could use something like #ifdef __APPLE__
…
#else
…
#endifto use |
|
I'm trying exactly this right now. The problem is that C++17 not only provides So if Apple could not be bothered to introduce the |
|
OK, I'm falling back to some emulation of |
Yes, I am not a numba expert and can not tell what level of numpy numba is able to digest.
I wasn't aware either until this morning, when I started googling because I couldn't believe that there wasn't any standard library for quaternion operations. I should say that I'm not sure it is mature enough to be considered standard but, skimming through the source, it seems to me that all the functionalities we need are there. Rotator is essentially a quaternion container with a bunch of methods for compositions, representation, rotation application etc.
From a quick look I believe that it could be useful already in the current implementation. I presume that broadcasting would percolate from
|
Mmm, I'm doing a few tests using broadcasting and the Introducing broadcasting without causing serious performance regressions does not seem trivial; at least, not for me. I don't have time to investigate this, and the Numba code I wrote so far works well (all the results look mathematically correct), so for the moment I'll leave everything as it is. We can always introduce broadcasting later, once we find the culprit. |
The "Observation" class should be completely unaware of the details of the
scanning strategy: it's not its duty to know that a spinning axis and a
precession axis exist at all. Moreover, the code as it was before this
commit was not efficient: every observation generated the
boresight-to-ecliptic quaternions, even if multiple observations could in
principle overlap the same time frame (e.g., an Observation for 30 GHz
detectors and another for 40 GHz detectors, both spanning the same time
interval) and thus have the very same set of quaternions.
This commit is clearly separates responsibilities and, although not as
efficient as it could, it enables a number of niceties and future
optimizations:
- The set of quaternions is generated by a method of the
`ScanningStrategy` class, no longer in the `Observation` class
(`ScanningStrategy.generate_bore2ecliptic_quaternions`). This is the
most obvious place where this computation should happen, and it does not
prevent other scanning strategies from being implemented (e.g.,
dedicated follow-up of calibration sources).
- The method `Simulation.generate_pointing_information` simply wraps the
`generate_bore2ecliptic_quaternions` above
- Because of the previous point, the `Simulation` class needs to
know when the simulation starts and how long it lasts. Therefore, two
new fields `start_time` and `duration_s` have been added to the
`Simulation` class and to its constructor (previously they were passed
to `Simulation.create_observations`). This is more elegant, as these
fields are stored in the `Simulation` class for future reference and are
included in the header of the report generated at the end of the
simulation.
- The method `Observation.get_pointings` now requires a set of
quaternions to be slerped. A simple quaternion matrix is not enough
here, because you want to know the time of the first quaternion (e.g.,
2027-01-01) and the time interval between consecutive rows of the
matrix, I have created a `Bore2EclipticQuaternions` class that collects
the three fields together. This class is returned by the method
`ScanningStrategy.generate_bore2ecliptic_quaternions` and is stored in
the `Simulation` object. I foresee that other classes in the future
could create this object, e.g.,
`PlanetFollowUp.generate_bore2ecliptic_quaternions`: this will be
compatible with the way this method `Observation.get_pointings` works,
which is a big advantage.
- It should not be difficult to parallelize the call to
`Simulation.generate_pointing_information`: each MPI process should
generate a subset of quaternions, which would be put together in a large
matrix at the end. This commit does not attempt to do so.
|
From what I could understand you are moving some data from the observations to other objects. I believe that it would be good the keep the observation as the only class responsible for the distributed time-domain or detector-domain data (actually I would use this as the definition of what the Observation class should do). The idea is to keep the complexity of chunking and distributing this information in one place (the Observation class) and have other classes (possibly functions) that take data from the observations and operate over it. What do you think? |
|
About quaternions routines, I'm fine with that. The scipy implementation doesn't look like the most efficient thing one can do and your test demonstrates it. What I had in mind was to write broadcast-compatible versions of your routines, but I'd keep it as a low priority thing. I'll propose a patch if I have time. |
I agree. In fact this is still the approach used in the latest commits. |
A few improvements:
- Types and default values are specified for all the parameters in the
constructor. This should make the documentation page for pointing
generation easier to read and shorter.
- The functions `read_detector_from_dict` and `read_detector_from_imo`
have been replaced with class methods, which are shorter to type, more
elegant, and they match the way class `ScanningStrategy` behaves.
Moreover, there are less names to be exported in __init__.py
Force the "pixel" to be an integer number and add a docstring.
The changes to the code in this commit have the only purpose to make the API easier to understand; all of them stem from the very act of having written the documentation in "docs/source/scanning.rst". A few examples: 1. The class `ScanningStrategy` has been renamed into `SpinningScanningStrategy` and a new base class `ScanningStrategy` has been introduced: this makes the development of alternative scanning strategies far easier 2. Related to point 1., a few lines of code that were put in the old `ScanningStrategy.generate_bore2ecl_quaternions` have been moved in new static methods of the base class `ScanningStrategy` 3. The method `Simulation.generate_pointing_information` has been renamed to `Simulation.generate_bore2ecl_quaternions`, which is more precise (no pointing information is computed in that method). 4. The `detector` class now accepts defaults for all its parameters, thus making the examples in the User's manual far shorter and clearer to explain.
The upgrade solves systematic crashes that were happening on some systems using llvmlite 0.33.
The "Instrument" class is used to define the orientation of the boresight. It permits to specify a tilt of the boresight as well as its phase angle around the spin axis, which means that now the framework is fully able to support a simulation of the three focal planes (LFT, MFT, HFT). This comes at the expense of moving the β parameter (boresight-to-spin-axis angle) out of the "ScanningStrategy" class and into the "Instrument" class; it solves an old inconsistency, as the angle β is not part of the «scanning strategy», being an hardware-specific parameter. The dependencies have been updated: 1. The "sphinxcontrib-bibtex" library has been added; it's used to insert references to other papers using BibTeX. 2. The "ducc0" dependency now refers to the branch quat_rot_right, as it implements a new function needed for the quaternion composition used by the code. Once the "quat_rot_right" branch will be merged into "ducc0", this dependency will need to be updated accordingly.
The new method permits to simulate the observation of pointlike sources in the sky. As it depends on a few more quaternion functions, I have moved all the Numba code dealing with quaternions in the new file `litebird_sim/quaternions.py`. Many docstrings have been improved, in particular those associated with Numba functions.
|
I believe the PR is now ready to be merged. The API will need to be revised once #42 is merged, but this can be done with a new PR. Please have a look at it, the documentation is available here: https://litebird-sim.readthedocs.io/en/pointings/scanning.html |
4 participants
This PR implements a pointing-generation code, i.e., a set of routines that take a scanning strategy definition as input and produce a set of direction in the sky and polarization angles as output. Once the PR is finished, I expect it to be used in the following way:
The method
generate_pointing_informationworks as follows:N x 4and using the scalar-last convention, which is compatible with the data layout used byducc(see github.com/litebird/ducc/pull/1).use_mjdhas been set or not in the call tosim.create_observations, the transformation to Ecliptic coordinates uses a barycentric true ecliptic coordinate system and the true Sun-Earth direction (using astropy.coordinates) or a quick-and-simple geometrical representation of the Ecliptic plane.PointingProvider.get_rotated_quaternionsimplemented inducc(see this example).Simulationobject and generating the pointings for just one observation usingObservation.generate_pointing_information. This can be handy for debugging and is already used in the unit tests.Things to do:
ScanningStrategyobjectuse_mjd=TrueSimulation.generate_pointing_informationso that it iterates over all the observations and callsObservation.generate_pointing_informationfor each of themduccto generate pointing information and polarization angles at the same sampling rate as the dataFind a way to fix the warnings produced by ERFA about «dubious years» (see github.com/astropy/astropy/issues/5809 for an explanation of the cause)