lmaurits / BEASTling

A linguistics-focussed command line tool for generating BEAST XML files.
BSD 2-Clause "Simplified" License
20 stars 6 forks source link

'BinaryModel' object has no attribute 'share_params' #258

Open Anaphory opened 4 years ago

Anaphory commented 4 years ago

Switching an old BEASTling config file I had from a binary covarion model to a general binary model, I get

Traceback (most recent call last):
  File "/home/gereon/Develop/BEASTling/beastling/cli.py", line 136, in do_generate
    xml = BeastXml(config)
  File "/home/gereon/Develop/BEASTling/beastling/beastxml.py", line 65, in __init__
    self.build_xml()
  File "/home/gereon/Develop/BEASTling/beastling/beastxml.py", line 97, in build_xml
    self.add_run()
  File "/home/gereon/Develop/BEASTling/beastling/beastxml.py", line 160, in add_run
    self.add_state()
  File "/home/gereon/Develop/BEASTling/beastling/beastxml.py", line 219, in add_state
    model.add_state(self.state)
  File "/home/gereon/Develop/BEASTling/beastling/models/binary.py", line 28, in add_state
    BaseModel.add_state(self, state)
  File "/home/gereon/Develop/BEASTling/beastling/models/basemodel.py", line 401, in add_state
    self.add_frequency_state(state)
  File "/home/gereon/Develop/BEASTling/beastling/models/binary.py", line 40, in add_frequency_state
[admin]
    if self.share_params:
AttributeError: 'BinaryModel' object has no attribute 'share_params'

Seems like we have neglected the Binary model for quite some time? Or is this my personal mess? Anyway, dropping this here is better than forgetting it later.

Anaphory commented 4 years ago

Also:

Traceback (most recent call last):
  File "/home/gereon/Develop/BEASTling/beastling/cli.py", line 136, in do_generate
    xml = BeastXml(config)
  File "/home/gereon/Develop/BEASTling/beastling/beastxml.py", line 65, in __init__
    self.build_xml()
  File "/home/gereon/Develop/BEASTling/beastling/beastxml.py", line 97, in build_xml
    self.add_run()
  File "/home/gereon/Develop/BEASTling/beastling/beastxml.py", line 163, in add_run
    self.add_operators()
  File "/home/gereon/Develop/BEASTling/beastling/beastxml.py", line 358, in add_operators
    model.add_operators(self.run)
  File "/home/gereon/Develop/BEASTling/beastling/models/binary.py", line 249, in add_operators
    BaseModel.add_operators(self, run)
  File "/home/gereon/Develop/BEASTling/beastling/models/basemodel.py", line 683, in add_operators
    self.add_frequency_operators(run)
  File "/home/gereon/Develop/BEASTling/beastling/models/binary.py", line 260, in add_frequency_operators
    for name in self.parameter_identifiers():
AttributeError: 'BinaryModel' object has no attribute 'parameter_identifiers'
lmaurits commented 4 years ago

Hmm, it's the base class for the Covarion model, right? I use that regularly and have not encountered any problems. But maybe I'm just not exercising it in the right way...

Anaphory commented 4 years ago

There's also a binary model in its own right, I just tried to run a binary non-covarion model and got this.

lmaurits commented 4 years ago

The CTMC model? That's another subclass of this, though, right?

Anaphory commented 4 years ago

Hum. That is very strange indeed. I just typed binary, not binaryctmc.

According to https://github.com/lmaurits/BEASTling/blob/master/beastling/configuration.py#L739-L765, I would expect that to throw a ValueError("Unknown model type 'binary' for model section 'vocabulary', and failed to import a third-party model.")

That means, I need to have a closer look on what exactly is going on under the hood. I hate rabbit holes.

xrotwang commented 4 years ago

Maybe there's a file binary.py in your cwd? See https://github.com/lmaurits/BEASTling/blob/ba264911ff591dbd3487a519c5faa24ec623fa3d/beastling/configuration.py#L755-L763

Anaphory commented 4 years ago

I expect config["model"] == 'binary', so config["model"].rsplit(".",1) == ['binary'], so ValueError: not enough values to unpack (expected 2, got 1) to be caught and replaced by ValueError("Unknown model type 'binary' for model section 'vocabulary', and failed to import a third-party model.")

(That bare except is not nice, though.)

xrotwang commented 4 years ago

Ah, ok. I see. Looks like you are in for a couple of prints.

lmaurits commented 4 years ago

Yep, that should probably just be except ImportError. Maybe the else at 755 should be elif config["model"].endswith(".py") if that's how we want to signal an attempt to use an external model?

Anaphory commented 4 years ago

As I read it, the code we have looks reasonable, and the errors I expect are ValueError (like in my case) and ImportError (mistyped some module name). No need to have a '.py' in the string.

It's probably something dumb on my end. I'll drop some breakpoint()s and print()s this evening.

lmaurits commented 4 years ago

Argh, sorry, you're right, I quickly misread that split as operating on a filename to remove the extension, not separating a module and class.