fix sign of w0wzCDM inv_efuncs pyx by nstarman · Pull Request #15049 · astropy/astropy

nstarman · 2023-07-09T21:29:34Z

And update the internal consistency tests.

github-actions · 2023-07-09T21:30:11Z

github-actions · 2023-07-09T21:30:17Z

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

nstarman · 2023-07-09T22:29:55Z

Failure in https://github.com/astropy/astropy/actions/runs/5502061341/jobs/10026067493?pr=15049 is unrelated.
This is a different bug, from how numpy.vectorize handles object arrays, uncovered by fuzzed tests. ~~Opening a separate issue.~~ Opened #15050.

cmarmo · 2023-07-10T07:53:24Z

cc @cmarmo

is that an issue with the auto-completion?

nstarman · 2023-07-10T12:23:00Z

cc @cmarmo

is that an issue with the auto-completion?

#15050? I don't believe so. Numpy vectorize is a convenient but not-particularly-efficient for-loop and it can cast to object arrays during the broadcasting. .age isn't very happy about that.

nstarman · 2023-07-19T00:27:35Z

On reflection, I'm moving the cdef double to another PR.

nstarman · 2023-07-21T16:52:55Z

@saimn, I moved the cdef double to #15076, is this PR ready?

saimn · 2023-07-21T17:06:27Z

#15076 seems fine, but for this one I have no idea about the sign ;).

saimn · 2023-07-21T17:07:17Z


    >>> cosmo.comoving_distance(0.5)
-    <Quantity 1982.66012926 Mpc>
+    <Quantity 1849.74726272 Mpc>


Is there a way to check those numerical values with another package/software ?

I can test this with Mathematica.
I'll add some tests.

Confirmed:

I'll add a test.

Same example is now a test, with the Mathematica code if this ever needs to be rerun.

Ping @saimn.

@saimn, I implemented a test.

@pllim, I've checked the calculation (see screenshot in this thread) and implemented a trivial test. I think this is an uncontroversial merge, if you feel comfortable!

Can you please rebase then, the CI should be green by now.

And update the the internal consistency tests. Signed-off-by: nstarman <nstarman@users.noreply.github.com>

Signed-off-by: nstarman <nstarman@users.noreply.github.com>

pllim

I don't grok cosmology but aconley approved, so I am giving the ✔️ by proxy. Thanks!

lumberbot-app · 2023-08-23T15:04:32Z

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

Checkout backport branch and update it.

git checkout v5.0.x
git pull

Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 46df1bbb31d61fe1d1895f6a05abac5e3c35d39c

You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #15049: fix sign of w0wzCDM inv_efuncs pyx'

Push to a named branch:

git push YOURFORK v5.0.x:auto-backport-of-pr-15049-on-v5.0.x

Create a PR against branch v5.0.x, I would have named this PR:

"Backport PR #15049 on branch v5.0.x (fix sign of w0wzCDM inv_efuncs pyx)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

lumberbot-app · 2023-08-23T15:04:49Z

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

Checkout backport branch and update it.

git checkout v5.3.x
git pull

Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 46df1bbb31d61fe1d1895f6a05abac5e3c35d39c

You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #15049: fix sign of w0wzCDM inv_efuncs pyx'

Push to a named branch:

git push YOURFORK v5.3.x:auto-backport-of-pr-15049-on-v5.3.x

Create a PR against branch v5.3.x, I would have named this PR:

"Backport PR #15049 on branch v5.3.x (fix sign of w0wzCDM inv_efuncs pyx)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

pllim · 2023-08-23T15:05:22Z

@nstarman , are you willing to do manual backport for both release branches? 🙏

nstarman · 2023-08-23T16:42:44Z

I can do that. It would be nice if the backport bot could open Issues if a manual backport is required. It would be a better reminder in my inbox. Can that be configured?

pllim · 2023-08-23T16:52:19Z

I think that is overkill, the backport bot already applies this label that you can check: Still Needs Manual Backport

nstarman · 2023-08-23T21:44:11Z

@pllim, on v5.0.x this changelog entry is deleted, should I keep it deleted?

pllim · 2023-08-23T22:13:10Z

Oh maybe because it was rendered into CHANGES.rst when bug fix release happened. I suspect same for v5.3.x. Unless @astrofrog or @saimn has a better idea, you will have to recreate the fragment? Or should we create a new fragment but for 15059 ? 🤔

…v5.0.x Backport PR #15049 on branch v5.0.x (fix sign of w0wzCDM inv_efuncs pyx)

…v5.3.x Backport PR #15049: fix sign of w0wzCDM inv_efuncs pyx (v5.3.x)

saimn · 2023-08-30T17:11:28Z

Sorry for the long time with response, vacation time ;).
Re-changelog: you shouldn't have modified an old changelog entry (for a PR that has been released since)... instead just add a new one for this PR (15049).

nstarman added this to the v5.0.7 milestone Jul 9, 2023

github-actions Bot added the cosmology label Jul 9, 2023

nstarman requested review from aconley and pllim July 9, 2023 21:40

nstarman added the no-changelog-entry-needed label Jul 9, 2023

nstarman marked this pull request as ready for review July 9, 2023 22:36

nstarman requested a review from a team as a code owner July 9, 2023 22:36

aconley approved these changes Jul 10, 2023

View reviewed changes

saimn reviewed Jul 10, 2023

View reviewed changes

Comment thread astropy/cosmology/flrw/scalar_inv_efuncs.pyx Outdated

nstarman requested a review from saimn July 15, 2023 15:07

nstarman force-pushed the cosmo-w0wz-pyx branch from f1e4efe to 1c1bdb0 Compare July 19, 2023 00:31

nstarman mentioned this pull request Jul 19, 2023

use cdef double over cdef #15076

Merged

nstarman added backport-v5.0.x labels Jul 21, 2023

saimn reviewed Jul 21, 2023

View reviewed changes

pllim added Affects-dev PRs and issues that do not impact an existing Astropy release skip-changelog-checks Tells bot to skip changlog checks and removed no-changelog-entry-needed labels Jul 24, 2023

nstarman force-pushed the cosmo-w0wz-pyx branch 2 times, most recently from edbb4f2 to 8460977 Compare July 30, 2023 20:32

nstarman requested a review from saimn July 31, 2023 04:42

nstarman force-pushed the cosmo-w0wz-pyx branch from 8460977 to ac50c42 Compare August 7, 2023 01:41

nstarman force-pushed the cosmo-w0wz-pyx branch from ac50c42 to 62dc222 Compare August 7, 2023 02:21

pllim modified the milestones: v5.0.7, v5.0.8 Aug 15, 2023

nstarman added 2 commits August 19, 2023 14:59

fix sign of w0wzCDM inv_efuncs pyx

6805512

And update the the internal consistency tests. Signed-off-by: nstarman <nstarman@users.noreply.github.com>

update changelog

d4626fd

Signed-off-by: nstarman <nstarman@users.noreply.github.com>

nstarman force-pushed the cosmo-w0wz-pyx branch from 62dc222 to d4626fd Compare August 19, 2023 18:59

filter occasional overflow warning

06bada8

Signed-off-by: nstarman <nstarman@users.noreply.github.com>

pllim approved these changes Aug 23, 2023

View reviewed changes

pllim merged commit 46df1bb into astropy:main Aug 23, 2023

lumberbot-app Bot added the Still Needs Manual Backport label Aug 23, 2023

nstarman deleted the cosmo-w0wz-pyx branch August 23, 2023 16:40

nstarman pushed a commit to nstarman/astropy that referenced this pull request Aug 23, 2023

Backport PR astropy#15049: fix sign of w0wzCDM inv_efuncs pyx

ae21168

nstarman mentioned this pull request Aug 23, 2023

Backport PR #15049: fix sign of w0wzCDM inv_efuncs pyx #15223

Closed

nstarman pushed a commit to nstarman/astropy that referenced this pull request Aug 23, 2023

Backport PR astropy#15049: fix sign of w0wzCDM inv_efuncs pyx

ce9deab

nstarman mentioned this pull request Aug 23, 2023

Backport PR #15049: fix sign of w0wzCDM inv_efuncs pyx (v5.3.x) #15224

Merged

This was referenced Aug 24, 2023

Backport PR #15049 on branch v5.0.x (fix sign of w0wzCDM inv_efuncs pyx #15226

Closed

Backport PR #15049 on branch v5.0.x (fix sign of w0wzCDM inv_efuncs pyx) #15227

Merged

pllim removed the Still Needs Manual Backport label Aug 24, 2023

pllim added a commit that referenced this pull request Aug 24, 2023

Merge pull request #15227 from nstarman/auto-backport-of-pr-15049-on-…

1b4fc0c

…v5.0.x Backport PR #15049 on branch v5.0.x (fix sign of w0wzCDM inv_efuncs pyx)

pllim added a commit that referenced this pull request Aug 24, 2023

Merge pull request #15224 from nstarman/auto-backport-of-pr-15049-on-…

afeeaa6

…v5.3.x Backport PR #15049: fix sign of w0wzCDM inv_efuncs pyx (v5.3.x)

bnavigator mentioned this pull request Sep 14, 2023

Cython3 chokes on astropy 5.3.3 (worked in 5.3.2) #15315

Closed

Uh oh!

Uh oh!

Conversation

nstarman commented Jul 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 9, 2023

Uh oh!

github-actions Bot commented Jul 9, 2023

Uh oh!

nstarman commented Jul 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmarmo commented Jul 10, 2023

Uh oh!

nstarman commented Jul 10, 2023

Uh oh!

Uh oh!

nstarman commented Jul 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nstarman commented Jul 21, 2023

Uh oh!

saimn commented Jul 21, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nstarman Aug 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pllim left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lumberbot-app Bot commented Aug 23, 2023

Uh oh!

lumberbot-app Bot commented Aug 23, 2023

Uh oh!

pllim commented Aug 23, 2023

Uh oh!

nstarman commented Aug 23, 2023

Uh oh!

pllim commented Aug 23, 2023

Uh oh!

nstarman commented Aug 23, 2023

Uh oh!

pllim commented Aug 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saimn commented Aug 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nstarman commented Jul 9, 2023 •

edited

Loading

nstarman commented Jul 9, 2023 •

edited

Loading

nstarman commented Jul 19, 2023 •

edited

Loading

nstarman Aug 19, 2023 •

edited

Loading

pllim left a comment •

edited

Loading

pllim commented Aug 23, 2023 •

edited

Loading