Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in PairParameter -- can't be fit! #593

Open
paulray opened this issue Dec 30, 2019 · 3 comments
Open

Bug in PairParameter -- can't be fit! #593

paulray opened this issue Dec 30, 2019 · 3 comments
Labels

Comments

@paulray
Copy link
Member

paulray commented Dec 30, 2019

WAVES are a new kind of parameter with a pair of values. I just tried to use it with fitting and it fails like this:

Traceback (most recent call last):
  File "ex_pn.py", line 52, in <module>
    f.fit_toas()
  File "/Users/paulr/src/PINT/src/pint/fitter.py", line 177, in fit_toas
    fitpv = self.get_fitparams_num()
  File "/Users/paulr/src/PINT/src/pint/fitter.py", line 97, in get_fitparams_num
    for k in self.model.params
  File "/Users/paulr/src/PINT/src/pint/fitter.py", line 98, in <genexpr>
    if not getattr(self.model, k).frozen
  File "/Users/paulr/src/PINT/src/pint/models/parameter.py", line 1165, in value
    return self.param_comp.value
  File "/Users/paulr/src/PINT/src/pint/models/parameter.py", line 1729, in value
    return self.get_value(self._quantity)
  File "/Users/paulr/src/PINT/src/pint/models/parameter.py", line 619, in get_value_float
    return quan.to(self.units).value
AttributeError: 'list' object has no attribute 'to'

This is demonstrated by examples/ex_pn.py in PR #587

I was trying to fit a model with WAVES to some data with pulse numbers as part of my fixes for pulse numbering, which didn't seem to work before.

@paulray
Copy link
Member Author

paulray commented Apr 27, 2020

Note that in addition to this, PINT counts degrees of freedom by the number of parameters with frozen=False, and this will be wrong for PairParameters. We need to rework WAVES to generate two parameters, I think. But then things like as_parfile() will need to know that the two parameters are coupled and write them out on one line.

@scottransom
Copy link
Member

We talked about the PairParameters issue a bit before on the telecons. I personally think they should go away. As for parfile lines, It would probably be really simple to have the parfile readers/writers combine those if necessary/requested, but we could write them one per line as a default.

@abhisrkckl
Copy link
Contributor

pairParameters were introduced for the Wave component. It will be superseded by the WaveX model (#1609).
We are not planning to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants