Add code for epoch propagation #8
Conversation
msdemlei
force-pushed
the
add-proper-motions
branch
from
b969690 to
ceb7017
Compare
|
Dear @msdemlei @gmantele, thank you for your contribution! We are going to put some fresh air into this repo. I'm ok with this patch but I'm not sure about version number. Should we increase it up to 1.2.2 because I've already merged the previous commit? I'm going to think about the versioning and releases. I will appreciate if you share some ideas how to maintain this repo. I have some doubts about adding some astronomy specific functions because PgSphere might be used in other areas. It think it may be a good idea to create another package like pgsphere-astro which will be based on existing pgsphere where all astronomical stuff may be implemented including linkage with SOFA library. |
There was a problem hiding this comment.
Dear @msdemlei, thank you for your contribution to the project. The recalculation of astronomical objects positions for the current epoch is definitely a useful feature. I think PgSphere was created for astronomical purposes as well. What I'm concerned is to keep PgSphere with the minimal set of functionality that can be applied in different areas, not only for astronomical purposes. PgSphere introduces new types to work with coordinates and areas on the sphere, some operations on these types, several indexing structures and functions. I believe it was the original project scope. This scope allows to use PgSphere in different areas where coordinate system on the sphere is used. Proper motions propagation is one of the big set of astronomical functions. To make a complete solution we should also add some other astronomical stuff that will greatly increase the scope of the project. I'm propose is to skip this pull request because it is out of scope. There is an idea to create a new project with astronomical specific functions that will be based on PgSphere (pgsphere-astro).
|
Dear @msdemlei, thank you for your contribution to the project. The recalculation of astronomical objects positions for the current epoch is definitely a useful feature. I think PgSphere was created for astronomical purposes as well. What I'm concerned is to keep PgSphere with the minimal set of functionality that can be applied in different areas, not only for astronomical purposes. PgSphere introduces new types to work with coordinates and areas on the sphere, some operations on these types, several indexing structures and functions. I believe it was the original project scope. This scope allows to use PgSphere in different areas where coordinate system on the sphere is used. Proper motions propagation is one of the big set of astronomical functions. To make a complete solution we should also add some other astronomical stuff that will greatly increase the scope of the project. I'm propose is to skip this pull request because it is out of scope. There is an idea to create a new project with astronomical specific functions that will be based on PgSphere (pgsphere-astro). |
|
On Tue, May 30, 2023 at 06:41:11AM -0700, Vitaly wrote:
@vitcpp commented on this pull request.
and functions. I believe it was the original project scope. This
scope allows to use PgSphere in different areas where coordinate
system on the sphere is used. Proper motions propagation is one of
the big set of astronomical functions. To make a complete solution
we should also add some other astronomical stuff that will greatly
increase the scope of the project. I'm propose is to skip this pull
request because it is out of scope. There is an idea to create a
new project with astronomical specific functions that will be based
on PgSphere (pgsphere-astro).
... which would be a fork? If so, I'd say that would be a major pain
in terms of maintenance and user confusion.
If you're thinking of a separate package that would depend on
pgsphere, that's not unreasaonable. But for what right now is a
single, self-contained function, it would IMHO be rather silly.
While I agree that once SOFA/ERFA became a build dependency, spinning
off an extra package *might* be preferable in terms of maintenance,
for this epoch propagation code (that, by the way, will be useful for
any sort of motion defined in spherical coordinates, so it's not like
there's no conceivable use cases beyond astronomy) that's not the
case. In fact, because it's just a few self-contained lines of code,
its cost in terms of package complexity is close to zero. On the
other hand, it solves a problem very relevant to the TAP services
that presumably make up a sizeable chunk of pgsphere deployments in
the real world, so it's not like this opens the door to put just any
old functionality into pgsphere.
So... I'd ask you to take another look and perhaps merge after all;
It would IMHO suck if we had a diverging "pgsphere-vo" fork.
|
|
@msdemlei it is great to get some feedback from you! Let's be in touch. So, you have a fork which you'd like to be in sync with postgrespro/pgsphere repo. I understand your problem. I agree there is no sense to create pgsphere-astro for a single function. What do you think if to create another branch in the repo and push your function into this branch (master-astro, for example)? Having such separate branch may help you to be in sync with this repo. We may merge the master branch into this branch on a regular basis. IMHO, I do not like the idea to put such function into the main branch because 1) it is out of scope, 2) there are no good tests, 3) users may require some other specific functions to be implemented in pgsphere that may negatively affect the project. May be, we should wait for one more opinions!? One more question, we would like to move all sources into src subdirectory and make optional healpix+moc compilation. How can it affect you? Despite of healpix+smoc is a nice stuff it is not required for all users but makes the build process more complex. |
|
On Fri, Jun 02, 2023 at 07:25:30AM -0700, Vitaly wrote:
What do you think if to create another branch in the repo and push
your function into this branch (master-astro, for example)? Having
such separate branch may help you to be in sync with this repo. We
may merge the master branch into this branch on a regular basis.
I'm not sure whether I'd consider this a good solution for the long
term, as I'd still expect all kinds of operational pain ("what branch
do we base our distribution packages on?"; "what branch do we
currently run?"). But as a temporary measure to gauge whether, say,
there is indeed measurable interest in having more features of this
kind that we may need to fend off to maintain focus, that's certainly
an option.
Personally speaking, I do not like the idea to put such function
into the main branch because 1) it is out of scope,
I think I still don't quite understand why you're skeptical about
whether epoch_proc fits the scope -- firstly, because I strongly
suspect that the overwhelming majority of the pgsphere deployments
out there will be in astronomy (do you have indications to the
contrary?), but then also because motion in polar coordinates is not
limited to astrononmy either. Admittedly, the current choice of
units is rather biased towards astronomy, but that we could change if
that helps generality.
2) there are no good tests,
If you tell me what problems you see with what's in sql/epochprop.sql,
I'll be happy to try and fix them.
3) users may require some other specific functions to be added into
pgsphere that may negatively affect the project.
I have complete sympathy for fighting feature creep, and perhaps we
indeed need to more carefully define pgsphere's scope. But for one,
epoch_prop's impact on the rest of the code base is negligible --
it's nicely isolated and has an almost trivial interface --, so I
don't see much of a risk of negative impacts on the rest of the
package, then it's really a rather essential functionality within
astronomy (as we get further and further away from "everything is
epoch J2000"), and finally it's not like people have been queueing up
to get all kinds of odd features into pgsphere in recent years...
May be we should wait for one more opinions?
Since the next Debian stable release (which is what I care about
most) is about two years in the future, there's no particular time
pressure here. But given that not terribly many people watch this
space, I suspect we'd have to actively solicitate further opinions.
One more question, we would like to move all sources into src
subdirectory and make optional healpix+moc compilation. How can it
affect you?
Cleaning up the source tree would be a valuable thing. Note,
however, that right now (and about until the end of June), Christoph
Berg is also working a bit on cleanups and some other things on
pgsphere, and in order to avoid too many edit conflicts, it would
probably be wise to postpone restructuring the tree to July.
Making the library compilable withough healpix is probably a good
thing; as long as MOCs don't drop out entirely, that won't give us a
headache.
|
|
As Markus said PgSphere seems to be really an astronomy project. As far as we know, nobody else than astronomy people replied or contributed to any PgSphere fork. Maybe at some time, when it is really clear for everyone it is an astronomy project (that's to say, no one from another domain than astronomy says something), I think it would be a good idea to write explicitly somewhere that this project mainly targets the astronomy domain. |
|
Creating a new branch Project scope
epochprop tests Cleaning up the project tree Healpix+smoc pgSphere for astronomers !? |
|
On Tue, Jun 06, 2023 at 01:41:15AM -0700, Vitaly wrote:
pgSphere doc contains the following description of the project
scope:
- input and output of data
- containing, overlapping, and other operators
- various input and converting functions and operators
- circumference and area of an object
- spherical transformation
- indexing of spherical data types
- several input and output formats
Hm... would it be too bad if we added "motion in spherical
coordinates" to that list?
Concerning cleaning up the tree... I would be happy to wait for
your changes but could you please share with me your plans? From my
@df7cb, can you comment?
I would propose not to do it. It is the dilemma like RISC or CISC
:) Furthermore, I think there is no sense to implement astronomical
functions from scratch. It would be better to create wrappers for
lib SOFA functions, for example. It would be better to create
pgsphere-astro contrib and make wrapper functions in this new
library.
But: who would build such a thing? I don't have the resources for
the forseeable future, and given the general activity here over the
last decade or so, I strongly suspect nobody else will commit the
necessary resources either.
So, in practice the question is *not*: Do we want to slightly soil
(perhaps; I still feel spherical motion to be entriely within the
purview of pgsphere) pgsphere or do we want a clean and pretty SOFA
wrapper for postgres (where the additional question is: is there a
serious use case for, say, ephemeris calculations within a
database?).
The actual question is: Do we add two positively harmless functions
(that, because one of them returns pgsphere points, will depend on
pgsphere anyway) to pgsphere or will people continue to do epoch
propagation in all kinds of wrong (or at least differing; cf.
https://wiki.ivoa.net/internal/IVOA/InterOpOct2022DAL/adql-astrometry.pdf)
ways? Given that pgsphere is behind the a large number of TAP
services out there, it is in a unique position to fix that. And
epoch propagation becomes more important every year we move away from
J2000.
So, perhaps we can find a pragmatic compromise, where we have epoch
propagation in pgsphere for now but say in the documentation
something like:
This an an experimental function that may be moved to a separate
extension focused on astrometry on short notice.
That way, we improve the situation now, but if and when a more
appropriate extension comes around, we can move it there without
breaking any promises.
What do you think?
|
|
@msdemlei I think we might put your function under ifdef. For example, introduce macro PGSPHERE_ASTRO that will enable astronomical functions (by default, it will be OFF). I think it will be a reasonable compromise. Can it be suitable for you? |
I, personally, think it is a reasonnable thing to do. Motion on a sphere, does not necessarily apply to astronomy. |
|
IMHO, cleanup is foremost required around the upgrade scripts handling. There's a dozen .sql.in files in the root dir, but there's two dozen more files in upgrade_scripts/, and the logic to puzzle everything together in the Makefile is really ugly. About the idea of trying to avoid making pgsphere astronomy-specific: With that argument, the MOC data type should not have been introduced at all, but yet it was since the existing pgsphere users find it useful. Perhaps that idea might help to decide which extra functionality should go in in the future. |
|
On Wed, Jun 07, 2023 at 02:11:14AM -0700, Vitaly wrote:
@msdemlei I think we might put your function under ifdef. For
example, introduce macro PGSPHERE_ASTRO that will enable
astronomical functions (by default, it will be OFF). I think it
will be a reasonable compromise. I will discuss it with my
colleagues.
Hm... I think that has about the same disadvantages as a separate
branch, and I converseley don't see what it would buy you. Where
would you see the advantage of such an arrangement? Would the plan
be that the function doesn't appear in the documentation then?
|
|
It would also have the problem that different installations with the same SQL-level extension version would contain a different set of functions. |
|
I think to put proper motion calculations under Transformations category is a reasonable compromise and to apply the PR. One of my colleagues proposed to generalize this function as well. It may be the development for the future. Give me please some time to discuss it with my colleagues. |
|
Dear All, I tried to gather some opinions of my colleagues-astronomers. I think to allow this patch but it would be reasonable to generalize it in the future, not only for astronomical purposes. @msdemlei Could you please adjust the PR to avoid existing conflicts? I could try to help you if you have no time. |
This bumps version to 1.2.1, too.
Also, fixing upgrade script generation.
msdemlei
force-pushed
the
add-proper-motions
branch
from
ceb7017 to
44edb5c
Compare
|
On Wed, Jun 14, 2023 at 07:19:34AM -0700, Vitaly wrote:
Dear All, I tried to gather some opinions of my
colleagues-astronomers. I think to allow this patch but it would be
reasonable to generalize it in the future, not only for
astronomical purposes.
Excellent, thanks, and I'll be happy to help with the
generalisations.
@msdemlei Could you please adjust the PR to avoid existing
conflicts? I could try to help you if you have no time.
I rebased to the current master and retained the version number
1.2.1.
|
(and fixing a duplicated line in the Makefile too trivial to warrant another rebase)
|
@msdemlei Thank you very much for fast response! I appreciate you are ready to help with generalizations. I think we come back to this task a little bit later. We have some plans to do some improvements in pgSphere and I would like to be in sync with you. In past we discussed that your colleagues would like to pull some cleanups in Makefile. Could you please clarify your plans because I have to wait for your changes and to postpone my changes to be in sync with you. On my side, the next thing I would like to do is to move all sources into src subdirectory. What do you think if I move all c/cpp/h files into src subdirectory? |
|
On Fri, Jun 16, 2023 at 06:37:40AM -0700, Vitaly wrote:
In past we discussed that your colleagues would like to pull some
cleanups in Makefile. Could you please clarify your plans because I
have to wait for your changes and to postpone my changes to be in
sync with you.
That would be as per
#8 (comment)
-- I think that's progressing as we speak.
On my side, the next thing I would like to do is to move all
sources into src subdirectory. What do you think if I move all
c/cpp/h files into src subdirectory?
Sounds great to me.
|
This PR lays the basis for pg_sphere-based functions for
epoch-propagating positions (i.e., compute their positions, proper
motions, parallaxes and radial velocities after some time based on their
values at epoch X under the assumption of linear motion).
This largely follows what is implemented at the Gaia archive,
https://gea.esac.esa.int/tap-server/tap. In particular, it follows
Lindegren's recipes from ESA-SP1200.
We might consider for a moment to employ SOFA/ERFA (and hence astropy)
-compatible methods, which take into account secular aberration and use
certain relativistic corrections. I'd say we shouldn't. For one,
having secular aberration while assuming linear motions of the objects
is somewhat inconsistent. Having relativistic corrections results in
differences in the few-muas range for objects we are likely to deal
with while making computations a lot more expensive.
We might also consider having a counterpart for ESAC's
epoch_prop_error function, which propagates errors and thus in general
produces fully populated covariance matrices. For now I would suggest
that this kind of math should happen outside of the database (but I
could be swayed).