This page is a repository of notes for the redesign of our WCS and Transform classes. Do not take anything here as final, and do assume that anything here can be added, deleted, or changed at a moment's notice.
Strawman class diagram
updated version from RO
For comparison, here is a class diagram for similar bits of AST:
We choose the name TransformGraph because it really is a graph: it has nodes (frames) and edges (transforms), and could be quite complicated. The minimum TranformGraph consists of one Frame, and no Maps. TransformGraphs are mutable, though their component frames and mappings may not be. Must any Frame in a TransformGraph have a path to any other frame? It would simplify the design if this was true.
A Frame (AST: Frame, GWCS: CoordinateFrame) is an immutable coordinate set that a transformation could go from or to. Frames are uniquely identified by a set of tags (e.g. {"meanPixel", "ccd4", "raft6", "Exposure153"}). We will have to standardize on a list of valid tag names.
A Map (AST: Mapping, GWCS: astropy.model) is a transformation that takes some numbers and returns some numbers, and that knows its domain and range (as numbers, but not what those numbers represent). Maps are functors (callable classes), with an inverse method to (if available) perform the inverse transformation. Maps may be either mutable or immutable: both have valid use-cases, and we will likely need immutable versions of many maps to ease parallelization. We need to decide how we will get the inverse map: map.inverse_map()? InverseMap(map)? while being aware that this is often non-trivial and likely non-analytic.
A possible method for composing mappings is to overload operator() to take a Map and return the composition f(g()) (possibly of type ComposedMap). This solves the question posed by e.g. ComposedMap(f,g,h): is that h(g(f())) or f(g(h)))?
A Transform is like an immutable TransformGraph with a start and end frame (it could be an immutable view, which might make the initial implementation easier). This makes it clear where you're going from and to, and is the thing one would generally operate on when working with a TransformGraph (which could be arbitrarily complicated). If you want to add or modify the transforms or frames, you work with the TransformGraph; If you want to use it, you connect() your desired start and end points to make an immutable Transform and use that. We're not totally happy with this name choice: we couldn't come up with a better one but liked this better than AbsoluteTransform.
FITS_WCS is a subclass of Transform that knows how to be persisted as a FITS file, and can be approximated as a SIP (or TPV or other?) FITS distortion. The *FITS methods are the only place in this whole structure were we think about the FITS WCS standard at all. Russell is uncertain if we really do want this.
connect(TransformGraph1,Frame1,TransformGraph2,Frame2,commonFrame,collapse=True): a function to "merge" two TransformGraphs at commonFrame and return a Transform with start=Frame1 and end=Frame2. Raises exception if there is no valid path.
Details about the methods on TransformGraph and Transform:
- approximate: find a (polynomial, affine, spline, etc.) transform that best approximates this absoluteTransform to a given level of precision.
- simplify: remove loops, identity maps, etc. and eliminate unnecessary frames between start and end. Both the TransformGraph method and function connect() always simplify() the before returning the resulting Transform.
- collapse: combine as many internal transforms as possible (either by composition or other mathematical combination), to have the fewest transforms to get from start to end. Both versions of connect() default to collapsing the resulting maps, but can be told not to for debugging purposes.
- as_transformGraph: returns a transformGraph from a Transform, so that you can manipulate it again. Would this be better as a TransformGraph constructor that takes a Transform?
Old notes
Jim: The goal of the design of AbsoluteTransform is to hide whether it is implemented as a view into a TransformGraph with a specific start and end point, or a copy of just the nodes and edges in the graph that connect its start and end point. If we start with an AST implementation, I imagine we'd start with the former, but we'd be free to switch to the latter if we move away from it.
Jim didn't like "Model", as it sounds too much like its related to a fitter. Mapping or Transform is better, and I don't think we have a particular preference there.
Russell: I would prefer to adopt standard terminology if we can. The standards I know of are AstroPy and AST. The former uses Model, the latter Mapping. Given Jim's objection to Model, I suggest Mapping. I can also see an argument for Transform since we already use it, but we have XYTransform and Transform and it could be hard to root out old usage if we totally change what Transform means.
Jim: Since we talked, I've been wonder if we should just move simplify into AbsoluteTransform's constructor. I can't think of a context in which one would want an unsimplified AbsoluteTransform.
Russell: I think we need to somehow support transforming between any two frames using the unsimplified path (e.g. a sequence of transforms from connecting frame to frame), as well as using an optimized transformation. The latter is usually preferred, but the former is wanted for testing and might also be preferred for one-off computations if simplification is expensive.
Transform and its subclasses may or may not be mutable, we weren't sure, and weren't sure it should be specified. I'm not wedded to the SphToSph, CartToSph subclasses, but I feel like they might be useful (I don't remember if you and I came to a decision about those).
WCS is a very particular subclass of AbsoluteTransform, going from pixels to sky. The *FITS methods are the only place in this whole structure were we think about the FITS WCS standard at all.
Russell: I am not convinced we need or want a special class for WCS. I originally thought it likely, but if we have a good API for TransformGraph then I hope it will suffice.
I think the "combine transforms and get a simplified result?" question in your notes would look like this:
graph3 = graph1.merge(graph2)
destPixelToSrcPixel = graph3.connect("pixel1", "pixel2").simplify().collapse()
Or, if you start with two AbsoluteTransforms instead of two graphs, you could do one of these two:
transform1.compose(transform2.invert()).simplify().collapse()
transform1.compose(transform2, invert=True).simplify().collapse()
I don't know that we need separate simplify() and collapse(), but they are rather different operations.
Jim: Thinking about it a bit further, if we don't move simplify() into the AbsoluteTransform constructor, then collapse() should first simplify(), so we'd never have to call them both.
Russell: I agree; I doubt it makes sense to have both simplify and collapse. I'm not even sure how they differ.
As I think about this, I don't know that Jim and I discussed what happens if connect(start,end) can find 2 paths between them. I guess it chooses the route with min(len(frames)+len(transforms))? But what to do there is not obviously clear with forward/reverse either, I don't think.
Jim: There are a ton of well-known algorithms for shortest paths in a graph (check out the Boost.Graph documentation for a list; I'm not sure if we want to use Boost.Graph, but it might be worth considering). Many of those allow you to define a metric for length other than just the number of edges/nodes in between, but I suspect that's still the metric we want (and using that metric implies that one never has to call simplify() after connect(), even if we don't put simplify() in the AbsoluteTransform constructor).
Jim: I think I'd recommend that we conceptually consider all edges in the graph to be bidirectional, though the graph edge implementation might not actually hold both directions until they're actually needed. In other words, we assume that any inverse transform that doesn't exist can be computed once we decide it's needed, and compute distances in the graph without concern for direction.
Russell: I am not convinced we need to worry about this. If AST already does it then we can use what AST does. I also think in most cases our graphs will be fairly simple with only one path connecting any two frames. So I'd hate to get hung up on this.
Design notes from Russell
- It seems a hassle to make users extract an AbsoluteTransform before they can transform anything.
- If we have TransformGraph itself support transformation, then do we need it? A transform with two frames is just a simple graph, so we could offer a method that returns a simplified graph connecting two points:
Transformations we need
See: - DM-5918Getting issue details... STATUS
28 Comments
Russell Owen
Identifying Frames in a TransformGraph is an issue. I think it is clear that every transform should have a unique identifier; in Python I would probably just use the object ID, and is is very easy to generate unique integer IDs in C++. However, it seems clumsy for users to have to use such IDs. I'd prefer that simpler names be available. Two examples:
Notice that in the first case we have only two pixel frames and in the second case we have two per CCD. In the first case it would be nice if the user could just say "real pixels" but in the second case they would have to also provide the name of the CCD. Tags (sets of strings) seems one way to handle this. For instance to get the "mean pixels" for CCD "3" one could specify ("real pixels", "ccd3"). Based on comments above it appears that @jim would be happier with a dict of keyword:value, e.g. {"ccd": "3", "frame": "pixels"}. I think either would work. The former is simpler, but the latter is a bit safer, especially for very complex transform graphs. Is it worth the extra complexity and work over tags? I personally doubt it, but could be convinced otherwise. @jim also strongly suggested that the values of the dict ought to not be forced to be strings (e.g. ccd is often an integer). However, in my opinion it is too messy to support arbitrary value types in C++. It can be done but it is not worth the hassle.
Jim Bosch
I'd be in favor of making this even more like the Butler syntax, and dealing with the C++ arbitrary-type problem by just using classes instead of trying to represent dicts there.
In Python, I'm thinking something like this:
In C++, you'd always use tag classes:
(I'm imagining that
icrs
is a global variable, since it could be).Russell Owen
I don't fully understand what you are proposing for the inputs and outputs of transforms. For instance when you transform a point, or list of points, to a sky frame, what do you get out? I suspect you are hoping for more than a list of points whose values you know to be radians.
AST and AstroPy are optimized for transforming collections of points. If we use AST then we must do this initially and we might as well embrace it. This suggests that we want a new kind of Coord that has coordinate system info and a collection of data. I think this is a good analog to AstroPy coords. Similarly, if we attach metadata to pixel positions and focal plane positions, an object containing that metadata plus a collection of points is probably best.
All that said, I hope we can keep the C++ simple. I think it is out of scope to implement units with conversions and such. If we can leverage AstroPy to get some of that on the Python side, that would be nice.
Jim Bosch
When we're transforming arrays of points, the inputs and outputs ought to be a data structure that combines an array of dumb point with a Frame, and when we transform individual points (this is important too!), we need to return a smart point that combines a Frame with a dumb point. I don't there's any fundamental disagreement on how this will work on the Python side, and I do think Astropy (either coordinates or quantities or both will help us there).
The question I was getting at here is whether the "dumb" points have the same compile-time type in C++ for both angular and non-angular coordinates. I don't want to use compile-time types to describe Frames in C++, as you would with a full compile-time unit system - that'd be a huge templated mess. But I was hoping we could at least maintain angle safety at compile time, and make the "dumb points" something like
Point<double,2>
orPoint<Angle,2>
in C++. Because those are different compile-time types, Transforms that accept or return them need to have different compile-time types (even if that's via templating). And I could see it being useful to have the template parameters being SphericalFrame and CartesianFrame, with all other Frames inheriting from one of those.Russell Owen
More on inputs and outputs of transforms. One obvious choice is a class akin to astropy.coordinates with some metadata and a list of positions. However, I do see a few subtleties that worry me:
Jim Bosch
What does "frames in parallel" mean conceptually? My gut reaction is that this sounds like one of the weirder aspects of AST's interface we do not want to keep, but I should reserve judgement until I understand what they're for.
I think my default choice for how to deal with Astropy coordinates and our own code both wanting to do conversions would be to have our own class that's conceptually like Astropy coordinate but with its own backend, and only provide interoperability with Astropy coordinates to the extent we can make them safe (even if that limits us to just supporting explicit views as Astropy coordinates).
As a side note: I think we need to start thinking of this as our own library that happens to have an AST backend, not something that tries to preserve all of AST's concepts or functionality.
Russell Owen
I think parallel frames are a useful way of displaying data. I'm not saying LSST needs them, but I don't want to block them either. My goals are:
So, with that in mind...I agree it would be nice to have Angle support and possibly even SphPoint or Coord for sky frames. I just wonder if it can be fit into AST compatibility layer. It would be nice to have, but the generality of AST may make it too difficult. For instance AST Frame and Map can have basically an arbitrary collection of inputs and outputs in an arbitrary order. AST cannot safely assume that a pair of sky coords is in the order RA/Dec. That doesn't sound like an issue for Angle, but it does for Coord or SphPoint. This suggests that Angle should perhaps be moved into the AST layer.
Also, I hope the inputs and outputs of a Map and Transform can be easily represented in Python as a numpy array (a simple 2d array or a structured array). I'm not sure how to reconcile that with Angle, though perhaps we could manage it with a structured array (presumably there is some way to interface such a thing to C++).
Jim Bosch
I still don't understand what parallel frames are. Do you have a link to AST documentation on them, or a two-sentence summary?
I think this is fundamentally opposed to what I thought RFC-193 - Getting issue details... STATUS was proposing (I suspect that goes for Robert Lupton as well), though your perspective may be more in agreement with Tim Jenness's. It's beginning to sound to me like that RFC never actually converged on this point, and may need to be reopened and possibly escalated if we can't reach agreement on it.
To be clear, I certainly agree we should not put anything specific to LSST-as-an-observatory in this code, but that's just good software design. I absolutely think the interface we want to define should depend on LSST types like
afw.geom.Point
, and the scope of that interface should be set by our requirements, not AST's current scope.I also have no objection to spinning off this new WCS library into an Astropy affiliate or otherwise reducing its dependencies on unrelated LSST code. But this should only be done after first spinning off all of the LSST code to which it is related (e.g.
afw.geom
), so we don't end up with two versions of those libraries. And because getting the generalized WCS up and running is a relatively high priority, I think it'd probably be better to start by implementing it within the stack.Russell Owen
Jim Bosch I emailed you a copy of the AST Programmer's Manual. I hope we aren't as far apart as you think. I don't want to redo the whole AST interface right now, but I want the work we do to naturally lead to the C++ rewrite (which was agreed to as part of the RFC) in a way that does not require rewriting too much of what we do now. Thus the desire to have much of what we do now be part of the new AST API or compatible with it.
Jim Bosch
Here are my thoughts from reading through the AST documentation (as helpfully directed by Russell).
"Features" of AST we explicitly want to hide:
Frame
is aMapping
: this just seems like bad OO design, though maybe it made a bit more sense in an object-oriented C environment where the development overhead for new classes is large.FrameSet
has a currentFrame
, and hence is aFrame
. Same reasoning as above.Features of AST we don't need and hence shouldn't try to maintain or support:
Frame
features geared towards plotting).Frames
andMappings
with dimensions != 2Major functionality we need to add on top of AST:
Frames
andMappings
should be immutable.Mapping
(akaTransform
, or vice versa) that knows its start and endFrames
.Mappings
that have parameters and can differentiate themselves with respect to those parameters, and a simplified composition engine for these. This is necessary to implement fitters for these transforms, and hence the code might live in jointcal instead of the WCS library, but it's closely related.Things I'm not certain about:
Frames
we want to support rather than support weirdFrames
directly.To me that suggests the following approach:
shared_ptr
for AST objects).Note that this does not produce a C++ reimplementation of all of AST; the resulting library has significantly reduced scope, and hence this may not be something that interests David Berry as a work package. But I think a full C++ reimplementation of all of AST along these lines is not something LSST should try to take on. That's only partly because it is a lot of work that we can't justify for LSST. I'm also very concerned that increasing the scope would negatively impact the C++/Python interfaces of the parts we do want (for instance, adding new Frame domains would require either new first-class objects to represent "points" in those domains (and hybrid domains!) or dropping the very much desirable use of first-class objects as inputs and outputs).
Doing a full AST C++ rewrite that doesn't use first-class objects for input and output would be a lot more feasible (but still probably not justifiable from a budget perspective, at least if LSST construction funds are used for all of it). And we could then use that as a backend for a library that does use first-class objects for input and output. I get the impression that this may be more along the lines of what Russell Owen is thinking (and maybe Tim Jenness?). The reason I'm not enthusiastic about this is simply that it doesn't gain us anything; I'm not too bothered by the fact that AST is a C library instead of C++ (it's plenty callable from C++, and hence usable as a backend as it is). In fact, it would quite likely be a step backward in stability, as the C version of AST is a mature library that's been in use in production environments for a long time.
Russell Owen
@jim I agree that type safety for inputs and outputs of `Transform` is important, and is the feature that worries me the most.
I think we will need to support Frames and Mappings with dimensions other than 2. We need 1-dimensional mappings for separable transforms, such as edge rolloff. We will be using a 3 dimensional mapping for radial transforms (transform 2 dim to unit vector plus distance, apply 1-d polynomial to the distance, transform back). I also think we may want to be able to use time as a dimension and a frame. I agree we are unlikely to use wavelength. I agree that many of our frames will be 2-dimensional (regardless of what goes on inside the maps), but I doubt they all will, especially if we use AST to help visualize data.
I think the plotting capabilities are likely to prove useful, but I agree we need not wrap them at this time.
Basically I think AST will be far more useful to us than you seem to feel. I feel if we wrap a very select subset in a way that restricts use of the rest of it, we will lose a valuable tool. That is why I am far more interested in doing the full AST C++ rewrite, and meanwhile providing a wrapper that emulates the API we want.
You raise a good point about stability as a concern in a C++ rewrite of AST. However, AST does have unit tests, which eases my mind somewhat. The main problem with keeping AST as a C package is that it is very hard to maintain. I wrote a new Map and it was much more difficult than it had to be and I would have failed without help from David Berry. We have additional maps that we need to write, e.g. for edge rolloff and tree ring distortions. We need a system that is easier to maintain and that we understand.
Getting back to data and type safety, John Parejko and I have an idea that I hope you will like. In the following I use `point` to mean a collection of floats (which might be x, y or RA, Dec or anything else). I will reserve `Point` (with a capital P) for `lsst.afw.geom.Point` (usually 2D but possibly 3D):
Things I worry about:
Jim Bosch
I agree that using
Frames
for type safety is the right approach, and I think that we can resolve the question of how to typeTransform
by having a thin templated shell over a polymorphic non-templated core, while makingTransformGraph
a non-template class with templated methods. I realize that's not a very good description, but I'd probably have to start writing some headers to make it clearer.What hadn't considered was having a
GeneralFrame
that also provides an untyped interface. I like that idea, and I think it is the missing link that lets us support more kinds of Frames than strongly-typed input and output classes.I think the lowest-level interface (but still a public one) should generally operate in-place, but we'll want convenience interfaces that copy as well. Designing the typed lists will be tricky but worthwhile; I think we may want to add features to ndarray and use some NumPy customization APIs to make it possible to use e.g.
ndarray::Array<Point,1>
in C++ and have that convert into a structured array in Python whose elements interoperate with scalarPoint
s.So I think this addresses the "no gain" argument I made against taking ownership of more of AST;
GeneralFrame
would let us get access to more ASTFrames
for rarer use without adversely impacting the more important 2-dFrames,
requiring a lot of extra work to strongly-type them, or exposing our dependence on AST. It doesn't address my scope/schedule/budget concerns involving a bigger C++ AST rewrite, but I'm not an arbiter of those kinds of concerns – I'll just caution that the question of the scope of the AST rewrite was punted in the last RFC, and hence we shouldn't spend too much time on any designs that depend on a larger-scope rewrite until that question is addressed by the people who are arbiters of those concerns. Happily, I'm now convinced that this design doesn't (yet) depend on the answer to that question.Jim Bosch
I've just attached a bit of C++ code that tries to explain at least some of what I was thinking in terms of how to use templating on
Frames
to give type safety toTransforms
. I haven't yet worked my way up toTransformGraph
, but I'll do that on the airplane tomorrow if I don't get a chance later today. As you can see, it's not at all free of templates, but it's not doing any crazy metaprogramming, and I think that's the most we can hope for here.John Parejko
I'm going to push even harder than Russell: we are quite likely to want a spectral frame for the photometric telescope (if nothing else, to be able to trivially persist/depersist data from it with the stack), and we're going to want to be able to plot coordinate lines and points over images.
Practically speaking, we're not likely to get any support from David Berry for a limited rewrite: he wants a new AST, not a new LSST-only thing. Plus, if we're going to go with DMTN-010's Option 1 (Develop our own), then we might as well just forget about AST. Which is silly, and why I rejected option 1 out of hand in the tech note.
If we do it generically and "right", we'll end up finding plenty of uses for it.
Jim Bosch
I actually liked Option 1 a lot (and I'm surprised you didn't think it was really worth considering). I did like the AST-backend option better (while assuming it was the limited-scope variant) because I thought it still get us to Option 1 eventually, while letting us get up and running faster and giving us the option of not migrating away entirely if we found that there were necessary pieces we didn't want to rewrite ourselves. I certainly agree that e.g. legacy persistence is a scary thing we don't want to write, but it's not clear how much we need it or would be able to rely on e.g. GWCS to do that part for us.
And while I agree we'd definitely want David Berry's help for a full-scope rewrite, I'm not convinced we'd want it for the more limited-scope rewrite I had in mind.
But I'm not the person who needs to be convinced here; as I said in my reply to Russell Owen's post, I think my technical concerns with building on top of a bigger AST rewrite have been addressed, and I'm not one of the people who makes scope/schedule/budget decisions.
Tim Jenness
I'm afraid I haven't really noticed this extensive conversation going on. I'm really concerned by the way the conversation is heading though. I don't understand why we can't end up with a product that is an AST rewrite containing the subset of AST functionality that we care about but which can be augmented by the community (with the option of also supporting spectroscopy for the calibration pipelines). My feeling is that porting mappings and frames from current AST to the rewrite should be fairly straight forward for others to do. Doing that makes it much easier for other applications to understand our WCS and we can't simply rely on GWCS doing everything else for us. There is a lot of power in AST that I feel people are not understanding.
Jim Bosch
As long as we're starting by defining our own interface and implementing it with as-is C AST (I think this is what the RFC settled), I think there's no fundamental disagreement on how we proceed for now (there was a possibility of that earlier in the discussion, when I was worried that trying to define an interface that was extensible to AST's full scope would adversely affect the interface for the functionality we know we need). I do think we have (at least the beginnings of) a design that won't close any doors, so punting the rewrite question as per the RFC is still ok.
That said, given that lots of people who have opinions are slowly discovering this page and wishing they'd known about it earlier, we really need to make this conversation somewhere more public. John Parejko, do you want to make an announcement somewhere, since it's your page? And should we start another RFC on the rewrite question now, even if we don't need an answer yet, so that conversation doesn't get in the way of the more immediate design?
Gregory Dubois-Felsmann
A couple of comments/questions, at a much lower level of sophistication than most of the discussions above:
(1) As part of the operations of the Observatory, there will be a need to take a 3-tuple of information from the OCS or TCS: ( ra, dec, skyRot ) - or ( alt, az, cameraRot ) together with an observation time - and generate a best-guess of the pixel-to-sky mapping resulting from that pointing. This could be used, for instance, to find the sky coordinates of the guide sensors in order to select guide stars and configure the guider readout ROIs. It would also be used to pre-compute references for Alert Production (using the 20 seconds of advance notice of the pointing that we are supposed to get). On the general theory that we should try to solve problems only once, it would be good to end up with an understanding of how to perform this simple operation. Would it be reasonable to do that with these new classes? A Camera description (nominal, or one refined by understanding of the as-built details of the geometry) is required in order to do the job. Where will the code running on the Summit get this description?
Gregory Dubois-Felsmann
One more little bit on this question: from a dependency-management perspective, how much of the stack will need to be part of the Summit code in order to support this operation? It would be nice to make this possible with a relatively lightweight installation.
Jim Bosch
I don't think there's much point in trying to limit the dependencies we install at the Summit, at least in terms of code like this. It's hard to imagine that the requirements for a lightweight install at the summit could be more restrictive than what a typical level 3 science user would tolerate, and all of this code is definitely in the low-level science pipelines primitives that they'll need.
Gregory Dubois-Felsmann
(2) The Firefly-based image visualization capability of the SUIT needs to be able to take in images from various sources (raw Camera images with no AP-generated detailed WCS yet, calibrated images post-AP, coadd cut-outs, etc.) and allow the exploration of the image contents in at least three coordinate systems: pixels, focal plane x-y, and sky coordinates. The tool already allows a live coordinate display to be generated for the location of the mouse pointer in an image as the pointer is moved around. In order to have this be a very low latency continuously-updating display, the coordinate transforms are computed in Javascript on the client on mouse-move.
Firefly is currently able to evaluate a variety of commonly-used FITS-based WCS types in this way.
We already have a notional high-level design, which emerged from recent conversations in various fora, for how this will evolve into a world in which the coordinate transforms are defined only by the APIs being discussed on this page, using LSST-specific external sources for the parametrization data that may not be expressible in any standard FITS header format:
As part of the preparation on the server side of the image pixels for display, which involves Google-Maps-style tiling of the image at the current zoom level, the standard LSST transforms libraries (i.e., the ones that will emerge from the present discussion) will be consulted for a zoom-level-appropriate approximate transformation for each tile. This should satisfy continuous boundary conditions from tile to tile and produce a parametrization of the approximate transform that can be serialized (e.g., as JSON) and sent to the client with the tile data for use in the Javascript code that evaluates coordinates on mouse-move.
(On mouse-pause, where a slightly greater latency is acceptable, the architecture allows for a callback to the server for a precise answer. Currently this is not needed, because the precise computation can be done in the existing Javascript implementation of the supported FITS WCS forms, but the callback hook is already there for other reasons.)
We need to be sure that the new coordinate transform libraries are capable of providing this service. I see the
approximate()
method is already planned; we will need to define the interface to facilitate the communication of the approximate transform to the client.Trey Roby
I have just been made aware of this conversation. I have not had time to study this. However, I would like to give a sanity check comment.
We can implement a virtually any WCS algorithm in JavaScript in a performant way. What we need is for it to be well documented and defined from the beginning. I am very concerned that the definition of this transform will be "whatever is implemented in python".
Jim Bosch
I think we'll be able to provide clear documentation for the coordinate systems used by any images we produce ourselves. It's coordinate systems that come from external datasets that users will want to combine with LSST that worry me more, but if you are already planning to have JavaScript support for the suite of FITS standard WCS, maybe that's nothing new.
If we need to have a fairly-complete JavaScript reimplementation of a transform composition engine and all of the concrete transforms we plan to use, that will be a significant amount of work, regardless of how well-documented it is (possibly more than the work we need to get the C++/Python interface going, since that can delegate to an existing library).
But I think we could instead use approximations to avoid all of that work in Firefly, if we could instead use C++ code to generate a discrete grid of coordinates on arbitrary scales that can be interpolated on the client side. For really demanding applications (likely just DM developers debugging their own code), we could use a grid that's as fine as we need; for most science users, having a grid point every few tens of pixels would likely be quite sufficient. And I suspect this would be much more performant than building a full transform composition engine in the common case.
John Parejko
Just a note for those who have just joined: the goal of the discussion on this page is to design an API, not to deal with broader questions of applicability or other use cases. Russell is going to produce an RFC for the API once we've hashed out some final details, and we can explore questions from those who might use the API there.
To the Firefly questions: I would think that exploring emscripten to wrap the transforms library might be well worth it, especially if we want to use something more than approximate transforms in Firefly.
Gregory Dubois-Felsmann
In response to your concern, I've moved the general dependencies question to c.l.o: https://community.lsst.org/t/dependencies-and-other-issues-in-using-dm-code-in-quasi-real-time-processes/903 . It's clearly a more general issue than the WCS/Transforms design.
With regard to the use of emscripten (and asm.js or Webassembly), I'd guess that that would tend to work better the smaller the code base that is required to be cross-compiled to support the function desired. But we can continue that on c.l.o, too.
Russell Owen
I think this summarizes the basic design, to date:
Map
Methods include:
Notes:
Frame
Transform
Attributes include:
Methods include:
TranformGraph
Type Safety
We want to support type safety for the data being transformed. This includes:
There appear to be two basic approaches:
Type Safety Via C++ Templating
In this model we have several kinds of frames, each with an associated class for a colletion of data,
and Transform becomes a templated class. The frames and data classes are:
The templating would look something like this:
Advantages:
Disadvantages:
Type Safety Via Runtime Checking
In this model we do no templating. Instead we only have one Frame class that acts like GenericFrame in "C++ Templating", and transformation accepts a single class for data input and output. That generic class would have enough information to allow runtime-checking (so that RA is not being fed to a Dec or cartesian axis).
However, we would still like to be able to express data as collections of spherical coordinates or cartesian points. In order to support that, we could offer classes that offer views into generic data. For instance SphCoordView and CartesianView. This adds an extra step when wanting to view data in this way, but is fully general in that a view can connect to any axes in a Frame.
Jim Bosch
I've updated my transformations.cc to include
TransformGraph
and a lot more detail. I may have gone...a bit...overboard (I blame a long flight with no distractions and a pent-up desire to write code after a lot of document-writing). So don't feel obliged by any means to take all of it - I'm guessing there's substantial overlap with stuff you have already worked out - but please do take a look; I was pleasantly surprised by how it all really came together. In particular, I think it has at least partial solutions for three of our bigger problems (some of which you've brought up above):Angle
to astropy quantities in Python, and maybe even consider a bigger compile-time unit system in C++.Map
mutability andTransform
immutability.Frame
lookup without actually needing separateTag
classes, by givingFrames
a hash function and using lazy-initialization for AST part of them. But I may be making some invalid assumptions here about how AST Frames are constructed.By the way, I think I can confirm that the AstroPy table data model is compatible (to first order, just think of astropy.table as a dict of NumPy arrays or Quantities). But I think I'd probably prefer just using structured numpy arrays (or structured Quantities, if such a thing exists) for return values, as I think tables are a much higher-level (and possibly higher-overhead) data structure.
Russell Owen
I suggest a change of course. First of all I suggest a minimal object-oriented shim around AST (at least the parts of it that we use), making the C++ look a lot like PyAST. Then we add any LSST-specific bits that we want around that. The advantages I see include:
The rest of this document describes the AST shim that I am proposing. I will talk about the additional LSST code separately.
Basic Features if a C++ AST shim
The following features are definitely wanted in the C++ shim for AST. I believe all of these already exist in PyAST:
ast
prefix on all names with namespaceast
and lowercase the initial letter for function and method namesAdditional Changes to Consider
The following changes might be worth also doing now:
Frame
not inherit fromMapping
. If this doesn't lose any crucial functionality it would probably be a small improvement in the design. I believeFrameSet
will continue to have to inherit fromMapping
, as changing that would break too much.tran
functions. This would be fairly easy, but departs from the existing API in ways that would be hard to describe.Features to Postpone
The following features should probably be postponed for a possible rewrite of AST in C++:
What Needs Wrapping
The following classes and their methods need wrapping
fit
andrms
vectors of doubleThe following free functions need wrapping:
We need the following classes for I/O. Start by wrapping them directly, but we may be able to do better in the long run with some higher-level code, e.g. constructors or class methods for appropriate objects for reading data.
Do Not Wrap
The following classes will not be wrapped until needed:
The following methods will not be wrapped until needed:
Our wrapper code performs all status checking and raises exceptions as appropriate, so we will not wrap status functions:
I hope that our code can call the begin and end macros. If not, wrap those macros:
The following free functions will not be wrapped initially
The following are function prototypes that will not be wrapped
Open Questions
Map
andFrame
need to be templated on the number of input and output parameters? I would like to avoid this if we can produce idiomatic C++ without it. Jim Bosch's existing worktransform.cc
may inform this decision, and if nothing else, proves that templating can be done in a reasonable way.LSST-specific Code
LSST will want to add its own wrapping code in order to support type safety by allowing collections of
Coord
andPoint
to be converted. Jim Bosch made strides in this area withtransform.cc
. I suggest doing this with as few LSST-specific classes as we can, so we use AST as purely as we can.