BioBlender icon indicating copy to clipboard operation
BioBlender copied to clipboard

Inconsistent parent object naming

Open zeffii opened this issue 9 years ago • 27 comments

All .pdb imports will result in a parent Empty with children atoms. Some .pdb imports seem to encounter a problem naming the parent object uniquely.

02_4IHVonlyDNA.pdb -> yDNA (coincides with modelRemark) 06_1L2Y_4GE.pdb -> _4GE.001 but modelRemark is _4GE

The .001 is added by blender in the following scenarios:

  • when an object is added with a proposed name which exists already in the bpy.data.objects collection.
  • an object is duplicated.

I only just noticed this being a problem now, so don't have immediate suggestions. This does interfere with automatic parent object selection for the Direct MLP mapping function. I would prefer to find and fix the circumstance under which .001 gets added than to write code that needs to do extra work to find the parent object.

zeffii avatar Feb 27 '15 12:02 zeffii

I think there are more bugs than this: each molecule should have an Empty (and it should take the name of the molecule). Instead there is only one Empty, and it includes all atoms of all moving molecules. The name of the molecule was supposed to be the last 4 digits of the PDB file. I have no idea of where the .001 comes from (but i think you do)

MonZop avatar Feb 27 '15 14:02 MonZop

this section of code is quite dense with features and side effects (meaning: certain lines of code have effect on things which are not explicitly mentioned, no mysteries just potential for increased clarity)

zeffii avatar Feb 27 '15 15:02 zeffii

The issue of an empty per molecule can be dealt with separately.

zeffii avatar Feb 27 '15 15:02 zeffii

weird bug.

If the last 4 characters of the pdb name (not counting the extension) are all alphabetic, then this issue doesn't seem to happen. In the case where the last 4 chars were _4GE it does, perhaps because of the underscore. If the pdb is renamed and extra characters are added for example making it 06_1L2Y_4GENV instead of 06_1L2Y_4GE, then the parent is correctly named and coincides with bbModelRemark.

This leads me to believe that the content of the file is not responsible, unless files get parsed differently depending on their full pdb name. Which doesn't (to me) seem likely, but is a possibility.

Anyway, this might mean the bug is localized and the code involved can be stepped through. No ETA.

The problem seems to be refinable to where the underscore is at the start of the last 4 characters of the name, or inbetween. ie

last 4 resulting empty name
s_wH s_wH
_wHR _wHR.001
_4GE _4GE.001

zeffii avatar Feb 28 '15 09:02 zeffii

I get the distinct sense that whoever coded core_createModels(): had a bit of a battle with BBModelRemark

# these right sides
a = copy.copy(str(bpy.context.scene.BBModelRemark))
b = str(bpy.context.scene.BBModelRemark)
c = bpy.context.scene.BBModelRemark

it might be justified, but it looks weird

zeffii avatar Feb 28 '15 10:02 zeffii

I have solicitated the writer of the code to provide some explanation and/or help. I am totally unable to contribute here

MonZop avatar Feb 28 '15 10:02 MonZop

copy.copy(x) Using a = copy.copy(x) means the value of x is anticipated to change, but a is intended to carry the value that was present at the time the copy.copy line was executed.

str(x) returns the original object if it was a String, or casts from one object to String if the object has a __str__() method, else it uses repr(x)


Any insight is appreciated, else it just takes time+effort to figure stuff out in hindsight.

I would elect to store the BBModelRemark (and other persistent info) in a .json file inside bpy.data.texts, but I won't change the current code without good reason.

zeffii avatar Feb 28 '15 11:02 zeffii

There's a neater way to add an Empty.

D = bpy.data
scn = bpy.context.scene

empty = D.objects.new('whatevername', None)
# empty.location = vert_coordinate
# empty.empty_draw_size = 0.45
scn.objects.link(empty)
scn.update()

I don't see anything preventing us from using this kind of Empty creation.

zeffii avatar Feb 28 '15 21:02 zeffii

I've added a single line fix for now, but in the EmptyTest branch. (it's possible to branch from a branch) The line that causes the rename is annotated here

        # on first model, Place atoms in scene
        if curFrame == 1:
            modelCopy = model.copy()
            # select and temporary rename template atom
            bpy.data.objects["atom"].hide = False
            bpy.data.objects["atom"].select = True
            bpy.data.objects["atom"].name = str(_id)   # <-------------------- here

There's nothing wrong with doing this, but i'd like to clarify the side effect in case anyone is reading who may be confused. This line

bpy.data.objects["atom"].name = str(_id) 

will name the "atom" object with the content of str(_id), but if another object exists with this name then the other object is renamed str(_id) + '.001' to avoid naming collisions. This is a side-effect, and it happens silently. I'm confident the original author of the code was aware of this, but for whatever reason at the end of core_createModels() it can happen that the pdbempty isn't renamed back to the plain str(_id).

I haven't tracked down why sometimes it is and sometimes it isn't, and why that should be affected by non alphabetic characters in the BBModelRemark is ..unfortunately.. still a mystery.

zeffii avatar Mar 01 '15 10:03 zeffii

Nice BioBlender is moving again. While it'd be nice to fix the root cause, it may be easier to ensure the object that needs to be accessed later on using the name property really is named without the appendum (.001,.002,...) (as zeffii said) such that it can simply be cut away when resolving the relating object. (Though it might be nicer/speedier to store a reference/address to the object, but that of course depends on the overall design, i.e. the object name persists when saving the blend file while the address changes from session to session.). I'm sure zeffii will find a way.

faerietree avatar Mar 03 '15 15:03 faerietree

I haven't tracked down why sometimes it is and sometimes it isn't, and why that should be affected by non alphabetic characters in the BBModelRemark is ..unfortunately.. still a mystery.

Only guessing, but what if blender internally replaces _ with <space>, i.e. when checking if an appendum like .001 is needed? It might just trim the underscore/space away temporarily if it was at the beginning or end like for your_4GE. Now if the rest of the string/identifier, e.g. 4GE was already given, then we'd have a conflict.

Though if 4GE is not used already, then there shouldn't be issues.

An alternative cause may be the _<number> combination and not the underscore alone (because identifiers are not allowed to start with integer by spec, probably to not get ambiguous, e.g. to know when <number>, e.g. 1 is an integer and when it's an identifier assigned to), but you tested _wHR too and it lead to _wHR.001, so that's out of the question.

It'd be interesting to know if e.g. 4GE_ or IGE_ as the last 4 characters also lead to the issue. Anyway, looks like an uncomfortable bug. Used the following code as a text block in blender for quick testing, but it works as expected. Then it's unclear where this bug comes from (it doesn't seem to come from blender side though).

import bpy
for o in bpy.data.scenes["Scene"].objects:
    if o.type == 'MESH':
        o.name = "4GE"
for o in bpy.data.scenes["Scene"].objects:
    if o.type != 'MESH':
        o.name = "_4GE"

faerietree avatar Mar 03 '15 15:03 faerietree

@faerietree I will wait for a while for the original author(s) to comment, they should be more intimately aware of the minefield involved in using these side effects.

testing what happens if BBModelRemark is AAA_ is quite easy.. let's see. 02_4IHVonlyDNA_.pdb gets DNA_ , and that's also the Empty name..

i'm more interested in getting it to a state where it is useful, then when I know how it works and what its supposed to do then perhaps (perhaps!) implement alternative code for speed or readability. We've discussed the possibility of doing Molecular dynamics externally, and perhaps caching / importing data per frame. But that's for a different thread.

zeffii avatar Mar 03 '15 16:03 zeffii

I confirm that I have solicited the original author again, and he said that he 'will se what he can do' when find some time.. Hopefully in the near future!

MonZop avatar Mar 09 '15 18:03 MonZop

in the meantime i've implemented it so it bypasses the issue, and would appreciate if you could test

zeffii avatar Mar 09 '15 18:03 zeffii

changed to : https://github.com/MonZop/BioBlender/tree/alt_manual_dup_and_mlp

zeffii avatar Mar 09 '15 19:03 zeffii

unfortunately for this branch (interesting as the experiment anyway) the game logic present in the Atom primitive is a bit tricky to create via python alone, so while it imports and plays conformations, the game engine logic isn't yet added. The python way that might work requires a certain context which is not present during the major enumeration of objects, i will give a shot at forcing a different context but if that fails then the existing duplication code is the only way currently to get around this problem, sadly.

Why it isn't possible to add sensors like this

# context agnostic
obj.logic.sensor_add()  etc..

instead of (the current)

# requires a context...
bpy.ops.logic.sensor_add(type="ALWAYS", object=obj.name)

zeffii avatar Mar 10 '15 09:03 zeffii

I am testing the latest version (BioBlender-alt_manual_dup_and_mlp), and I found that with the glycoprotien (test 05_glycoprotein_4AY9_FSH), the surface is calculated, but sugars are left out. surfacetest_05

Since PyMOL does include all atoms, I suppose that somehow the file that Blender sends to pyMOL las lost the sugars: maybe because they are not ATOM but HETATM in the original pdb file. Also the MLP does not work for me: I get a uniform grey surface (sugars excluded) in the viewport, and the green/blue/red version when i render. I will keep testing other features.

MonZop avatar Mar 10 '15 12:03 MonZop

As @zeffii mention, the GE does not evaluate collisions atomcompenetration

When surface MLP is requested: the surface is calculated, but it shows uniform grey, (i also checked the atomic MLP, and that seems to work OK) all atoms of the molecule are hidden (this is probably OK, just mention) mlp_outliner

MonZop avatar Mar 10 '15 12:03 MonZop

Current Tree structure.

image

At this point I'm confident that the BioBlender-alt_manual_dup_and_mlp will not work as I had hoped, but thanks for testing! I will ditch the branch (but save the code, it may prove useful some time later)

Re: Atom vs HETATM, let's see..

zeffii avatar Mar 10 '15 13:03 zeffii

As you can see, the tree is not really a Tree at the moment, but more a branching of a branch. The only two on that list that I really expect and hope to work are master and master_mff_experiments.

The pdb being sent to pyMOL does include the HETATM lines. This is a section of the code which I have not changed. To test if stuff is broken by new code, best way to go is to go from master then master_mff_experiments , and if there's a difference between those two then I can narrow down the lines of code which are different between the two branches.

zeffii avatar Mar 10 '15 13:03 zeffii

I just checked also Master and the other versions: they all do not include sugars in the surface.

MonZop avatar Mar 10 '15 16:03 MonZop

Oh, it is pyMOL that for reasons that I have to understand, does not include the sugars in the surface. OK, one less problem of BioBlender, sorry to have raised an issue that really does not belong here. pymolsurface

MonZop avatar Mar 10 '15 16:03 MonZop

@MonZop about the GE evaluation collisions, this could be something I introduced in the higher branches of that diagram (due to taking a different approach to creating the atoms).

Finding the source of the problem uses the same technique as described earlier. Test the earliest version first, if it works there, then the next..and so on, until it stops working. But if it doesn't work in master, or master_mff_experiments then it was already broken.

zeffii avatar Mar 11 '15 07:03 zeffii

I just tested the import: it works OK for all versions except the last: BioBlender-alt_manual_dup_and_mlp In this last version, the first molecule is imported OK. When trying to import a second one, instead of importing the new molecule, BB builds another atom and print an Error: import error

MonZop avatar Mar 11 '15 14:03 MonZop

I never tried that, but it shouldn't have to ' apply modifiers ' the the atom primitive more than once.

zeffii avatar Mar 11 '15 15:03 zeffii

I've now removed this branch, the code will still be available for browsing

zeffii avatar Mar 11 '15 15:03 zeffii

I think you have removed the previous one: the alternative_MLP_with_EmptyTest worked OK the alt_manual_dup_and_mlp does not import the second molecule

MonZop avatar Mar 11 '15 15:03 MonZop