I'm not in favour of this.

The warning was added once lal solved an issue we (and others) had long complained about, specifically that if a C-level error occurs, we only get some generic warning, with no real idea of what wen wrong, at the python level. This is now enabled by default in notebooks, but doing so does come with a performance penalty. I think PyCBC users might have cases where it is best to use with lal.no_swig_redirect_standard_output_error(): and cases where not, but this is hidden if we silence the warning intended to advertise this.

I've no issue with silencing this warning in tutorial notebooks (with a comment), but am not sure it should be put here (silently making this decision for any user that imports PyCBC).

@spxiwh I don't understand the choice of defaults here. Why is it different between a notebook and a script? A notebook is exactly where I wouldn't care as much about performance, so why the scary message that pops up?

I don't think the right solution anytime there are potential user environment choices to have a forced warning. If all packages did this, it would be untenable.

Wouldn't the better thing to be do have it off by default, but if an error occurs they get the complement of this message on how to get a more detailed error report?

"Why is it different between a notebook and a script?" -> A script (or python, or ipython window) will not swallow error messages raised at the C level. A jupyter notebook will. So if (for example) I try to generate SEOBNR and don't have the frequency high enough to include ringdown, the python error is just "RuntimeError: Internal function call failed: Input domain error", whereas the C error will tell you what the problem is. In the script you see it all, in Jupyter you do not. ... You will not see this warning from within a script.

Your suggestion at the end makes sense to me, but I don't know enough about SWIGLAL to comment on why it wasn't done that way, or if it is possible.

@spxiwh My guess is that it just wasn't suggested, probably not anything deep into any choice. I think the fact that swiglal was already able to raise a RuntimError with a generic "failed" message means that it should be possible. In fact, we could probably implement it ourselves.

For example, we could

(1) disable the error by default (but have some way to re-eanble?)
(2) put the lal* libraries uses in a try/except whereby the runtimeerror is caught and a message on how to enable a more detailed message is raised (if not already enabled).

What do you think?

@spxiwh I'm getting back to addressing this. Do you have an example where the c-error would not be apparent in a notebook? The obvious example of EOB doesn't seem to be a problem (or possible the disabling of the io redirection doesn't work).

For example running the following in a notebook seems to produce a sensible error,

import lal
from pycbc.waveform import get_td_waveform

with lal.no_swig_redirect_standard_output_error():
    hp, hc = get_td_waveform(approximant="SEOBNRv4", mass1=10, mass2=10, delta_t=1.0/100, f_lower=10)

This produces

XLAL Error - XLALSimIMRSpinAlignedEOBModes: Ringdown frequency > Nyquist frequency!
At present this situation is not supported.
XLAL Error - XLALSimIMRSpinAlignedEOBModes (LALSimIMRSpinAlignedEOB.c:1285): Input domain error
XLAL Error - XLALSimIMRSpinAlignedEOBWaveformAll (LALSimIMRSpinAlignedEOB.c:3912): Internal function call failed: Input domain error
XLAL Error - XLALSimIMRSpinAlignedEOBWaveform (LALSimIMRSpinAlignedEOB.c:783): Internal function call failed: Input domain error
XLAL Error - XLALSimInspiralChooseTDWaveform_legacy (LALSimInspiralGeneratorLegacy.c:1275): Internal function call failed: Input domain error
---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
Cell In[22], line 4
      1 from pycbc.waveform import get_td_waveform
      3 with lal.no_swig_redirect_standard_output_error():
----> 4     hp, hc = get_td_waveform(approximant="SEOBNRv4", mass1=10, mass2=10, delta_t=1.0/100, f_lower=10)

File ~/projects/pycbc/pycbc/waveform/waveform.py:599, in get_td_waveform(template, **kwargs)
    597     required = parameters.td_required
    598 check_args(input_params, required)
--> 599 return wav_gen(**input_params)

File ~/projects/pycbc/pycbc/waveform/waveform.py:180, in _lalsim_td_waveform(**p)
    177 #nonGRparams can be straightforwardly added if needed, however they have to
    178 # be invoked one by one
    179 try:
--> 180     hp1, hc1 = lalsimulation.SimInspiralChooseTDWaveform(
    181            float(pnutils.solar_mass_to_kg(p['mass1'])),
    182            float(pnutils.solar_mass_to_kg(p['mass2'])),
    183            float(p['spin1x']), float(p['spin1y']), float(p['spin1z']),
    184            float(p['spin2x']), float(p['spin2y']), float(p['spin2z']),
    185            pnutils.megaparsecs_to_meters(float(p['distance'])),
    186            float(p['inclination']), float(p['coa_phase']),
    187            float(p['long_asc_nodes']), float(p['eccentricity']), float(p['mean_per_ano']),
    188            float(p['delta_t']), float(p['f_lower']), float(p['f_ref']),
    189            lal_pars,
    190            _lalsim_enum[p['approximant']])
    191 except RuntimeError:
    192     if not fail_tolerant_waveform_generation:

RuntimeError: Internal function call failed: Input domain error

Am I misunderstanding something or does this only happen for other functions? Do you have an example on hand?

@ahnitz I don't see this behaviour. For me:

with lal.no_swig_redirect_standard_output_error():
    hp, hc = get_td_waveform(approximant="SEOBNRv4", mass1=10, mass2=10, delta_t=1.0/100, f_lower=10)

does not show the XLAL error, (although it does appear to globally disable the error messages and not temporarily do it as suggested).