Skip to content

add submodule for postprocessing#246

Merged
vijayvarma392 merged 19 commits into
mainfrom
posterior
Apr 13, 2026
Merged

add submodule for postprocessing#246
vijayvarma392 merged 19 commits into
mainfrom
posterior

Conversation

@md-arif-shaikh
Copy link
Copy Markdown
Collaborator

@md-arif-shaikh md-arif-shaikh commented Mar 24, 2026

Mains files to review:

  • core.py
    • Contains core stateless functions and dataclasses to be used by both PostProcess class and MPI based postprocessing
  • postprocess.py
    • Base class for postprocessing on local machine, for example from a notebook.
    • Designed in a way to make it easier for working with posterior generated by any PE pipeline and not only Bilby. Keeping in mind that we may test it also for LISA
  • postprocess_for_bilby.py
    • Derived class PostProcessForBilby for working with Bilby posterior
  • postprocess_cli.py
    • Script to perfrom postprocessing on cluster using MPI

Example dir contains notebooks and scripts to showcase postprocessing Bilby posterior.

@md-arif-shaikh md-arif-shaikh marked this pull request as draft March 24, 2026 18:15
@md-arif-shaikh md-arif-shaikh marked this pull request as ready for review April 7, 2026 08:59
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the examples live with the other examples here? Otherwise, anyone that installs gwecc will get these examples in their python dirs. I'm thinking a postprocessing directory in examples, along with a mention of this and what it does in the main Readme.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the example notebooks, it looks like there are huge outputs of warnings. We need to say something in the notebooks about this: what should people make of them, should they ignore them, etc.

Copy link
Copy Markdown
Collaborator Author

@md-arif-shaikh md-arif-shaikh Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, agree. Moved the examples to top level examples/postprocessing. Added a subsection in the Usage section in the main Readme.

Yes, I have cleaned up the notebook. Have another look, it looks much better. Earlier there were two sources for long outputs -- one was because I was using posterior for lower eccentricity and some of the samples were failing (because I was using Amplitude). Others were coming from rational_fit. Changed both and the notebook looks much more cleaner now.

@@ -0,0 +1,109 @@
import numpy as np
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explain why this file exists at the top?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Also renamed the file.


A wrapper around ``gw_eccentricity.measure_eccentricity`` to measure
eccentricity from the waveform modes for a sample with given ``params``.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the main function the user sees?
Whichever function that is, could you describe the entire procedure in the docs? Like how the frequency ranges are chosen, etc.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Shall I expand the docs for the params? or are you suggesting for a detailed description at the top of the function docstring?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some details on the range of allowed tref/fref at the top.

@md-arif-shaikh
Copy link
Copy Markdown
Collaborator Author

@vijayvarma392 could you have another look?

@vijayvarma392 vijayvarma392 merged commit 2a3ee53 into main Apr 13, 2026
3 checks passed
@vijayvarma392 vijayvarma392 deleted the posterior branch April 13, 2026 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants