MonZop / BioBlender

AddOn for Blender to do molecular work
BSD 2-Clause "Simplified" License
113 stars 20 forks source link

The Rewrite: BB2 Redux #32

Open zeffii opened 9 years ago

zeffii commented 9 years ago

Over the next couple of days (unless something unforeseen happens) I have time to make a start with building the add-on from scratch. Getting it to match the working features of BB2 will take a while. It's difficult to put a time frame on it until the process gets rolling.

The decision to start from scratch is not done in haste. I think some of the responders on this repository have expressed similar feelings about how to proceed. There's lots that can be taken from the current code-base, but a lot that I really want to sanitize and arrange into separate files. The core of this was written when Blender 2.5 was still young and the API not very well defined, and prone to convoluted solutions.

If I can reach feature parity and structure the code better then I will consider it a success, speeding up the import (and avoiding recursive updates) is not my first priority, making it easier for others to contribute is far more important.

Ticks indicates functionality seems to be restored for that panel, certainly report any issues with those sectons. The unticked sections have had no major checks done other than suppressing import errors. The section labelled next is either being worked on or that's where the most recent code changes have taken place.

zeffii commented 9 years ago

the rawID is the index of the vertex in the mesh, it gets converted to the coordinate, and that is checked against the nearest grid cells in the .dx (using a mulitplication and a rounding downwards to get the integer value, which corresponds to a 3d cell coordinate.....)

technically it isn't nearest grid point..(or not always nearest)

mcallieri commented 9 years ago

that code find the cell number in the grid. floor() is correct because you just need the INTEGER part of the division. look at this example, just the X axis: origin is X=0, xdelta=1, vertex1 is X=3.49 vertex2 X=3.51.... using floor you correctly find that both vertices are inside cell number 3 (remember indexes are 0-based)... with round(), vertex1 is still inside cell 3 but vertex2 jumps inside cell number 4....

The code I originally wrote was the simplest:

then I introduced another step to avoid banding

I do not know which version has been ported to Python

mcallieri commented 9 years ago

I also used the vertex + normal offset to avoid "noisy" values near the surface, But I used it in a later stage (when calculating the force field lines).

zeffii commented 9 years ago

Thanks for you explanation btw, it is becoming more clear. I suspect I will have to dissect the function to appreciate what it does and how it does it and why.

MonZop commented 9 years ago

I am not sure if this can help, but I noticed that the banding (moirè) effect is related to the size of the grid: the 2 images show renders done with MLP calculated with size grid of 1 and 2. mlp1 mlp2

zeffii commented 9 years ago

http://pymolwiki.org/index.php/Surface_quality (currently the Surface is generated with surface quality 1, but 2, or 3 might be better (slower, but better).

@MonZop Is there a source dump of the C++/C patched Blender, so I can read it? I recall seeing it but it must have gone down with the ship (recent HD failure).

zeffii commented 9 years ago

Maybe i'm missing something.. so here's to asking.

If the coder who ported that getVar function to python never managed to get it working because lack of access to the vertex index from the _vertexcolor index, then it was never tested? The algorithm is close. Or it did once work in python, and due to API change it was never reimplemented.

Pushing vertex coordinates in the direction of their normal by some amount doesn't really work either. The results from reversing the vertex color assignment are better. I'd prefer to get a hand on one of the algorithms in C that did work, before sticking on my own interpretations.

@MonZop The grid spacing can go much lower than 1.0 :) but there's a massive slowdown due to Cubic increase in calculations..

zeffii commented 9 years ago

See: image

the bug is probably an oversight, rather than anything else

MonZop commented 9 years ago

@zeffii You can downolad the code, included in the BioBlender 0.6 here I am not sure what happened with the transition from BB0.x to the 1.0 version. To my knowledge it never worked. Once again, I can ask the coder, maybe this time he will provide an explanation.

zeffii commented 9 years ago

I looked but didn't find the C equivalent code for the getVar() function. I still don't know the solution for the banding/stripes but it doesn't look like the mesh precision is an issue, rather how the getVar function decomposes the Surface vertex index into a shading value. The banding is happening on the Y axis it's probably a hint.

zeffii commented 9 years ago

OK. maybe this:

        v = ob.data.vertices[rawID].co
        dimx = dimension[0]
        dimy = dimension[1]
        dimz = dimension[2]
        deltax = delta[0]
        deltay = delta[1]
        deltaz = delta[2]
        cellx = int((v[0] - origin[0]) / deltax)
        celly = int((v[1] - origin[1]) / deltay)
        cellz = int((v[2] - origin[2]) / deltaz)

        mmm = dxData[cellz + ((celly) * dimz) + ((cellx) * dimz * dimy)]
        pmm = dxData[cellz + ((celly) * dimz) + ((cellx + 1) * dimz * dimy)]
        mpm = dxData[cellz + ((celly + 1) * dimz) + ((cellx) * dimz * dimy)]
        mmp = dxData[cellz + 1 + ((celly) * dimz) + ((cellx) * dimz * dimy)]
        ppm = dxData[cellz + ((celly + 1) * dimz) + ((cellx + 1) * dimz * dimy)]
        mpp = dxData[cellz + 1 + ((celly + 1) * dimz) + ((cellx) * dimz * dimy)]
        pmp = dxData[cellz + 1 + ((celly) * dimz) + ((cellx + 1) * dimz * dimy)]
        ppp = dxData[cellz + 1 + ((celly + 1) * dimz) + ((cellx + 1) * dimz * dimy)]

        wxp = 1.0 - (fabs(v[0] - (origin[0] + (deltax * (cellx + 1))))) / deltax
        wxm = 1.0 - (fabs(v[0] - (origin[0] + (deltax * (cellx))))) / deltax
        wyp = 1.0 - (fabs(v[1] - (origin[1] + (deltay * (celly + 1))))) / deltay
        wym = 1.0 - (fabs(v[1] - (origin[1] + (deltay * (celly))))) / deltay
        wzp = 1.0 - (fabs(v[2] - (origin[2] + (deltaz * (cellz + 1))))) / deltaz
        wzm = 1.0 - (fabs(v[2] - (origin[2] + (deltaz * (cellz))))) / deltaz

        onz_xmym = (wzp * mmp) + (wzm * mmm)
        onz_xpym = (wzp * pmp) + (wzm * pmm)
        onz_xmyp = (wzp * mpp) + (wzm * mpm)
        onz_xpyp = (wzp * ppp) + (wzm * ppm)
        onx_yp = (wxp * onz_xpyp) + (wxm * onz_xmyp)
        onx_ym = (wxp * onz_xpym) + (wxm * onz_xmym)
        val = (wyp * onx_yp) + (wym * onx_ym)
        dxCache[rawID] = val

can you spot the difference?

zeffii commented 9 years ago

image

zeffii commented 9 years ago

I have pushed this to BB2_Redux_s3_FIRE_MLP , if you can confirm that this does what it should have done all along I'll continue with the next things..

MonZop commented 9 years ago

Great! I have no idea how you did iut, but sure it seems to work. I'll test right now with other molecules. Thanks

MonZop commented 9 years ago

rendercam renderdna20150527 I would say that it works very well!

zeffii commented 9 years ago

OK, MLP_Render() function is next..

MonZop commented 9 years ago

Oh, these images are renders: i did 'show MLP on surface' and then Render MLP to surface, and then F12 to obtain the images

MonZop commented 9 years ago

You may go for something easy: the Surface BioBlender view, for example, only needs to calculate the surface (which is done also for the MLP, but it can be done even if one is not interested in the MLP).

zeffii commented 9 years ago

ok, i've started on the View panel, i see it doesn't generate A surface when the mode is set to that. Will have to poke around, it's not immediately obvious where it's broken. but should be harmless.

zeffii commented 9 years ago

Have rebranched : BB2_Redux_s4_VIEW it picks up where FIRE_MLP left off.

candidate lines for a rewrite . parking this here because it sticks out in my text editor..

if not (atomName == N or atomName == C or (atomName == CA and elementName != CA) or (atomName in NucleicAtoms) or (atomName in NucleicAtoms_Filtered)):

No formal approach yet, will add debug statements tomorrow.

zeffii commented 9 years ago

what exactly should the Surface ->Apply do, import the Surface without the vertex_colors? of the whole molecule?

zeffii commented 9 years ago

ok, I had missed an import, now View->Surface->Apply does generate the surface..

There are a couple of globals which I have not yet addressed in this file. i am aware of this - but haven't decided on what is a nice way to avoid those globals.. or fully understood why they are needed. This may seem trivial, and it might be, but I don't know yet.

MonZop commented 9 years ago

Hi, I confirm that Surface works! Sorry I cannot help with the globals issue.

zeffii commented 9 years ago

i'm not working on this a lot at the moment @MonZop -- but it is at the back of my mind - i've solved the globals 'virtually' just not coded yet. Soon.

The latest developments in the "dependency graph refactor" of Blender might result in a speedup for the rigidbody simulation setup..

Either way, having this all split up into modules again is much more pleasant to work with.

MonZop commented 9 years ago

Gald to know that you have some space for BioBlender, however small! Sorry to inform you that i just noticed that it is not possible to import motre than one pdb get_new_PDB_id Import Failed: Can't convert 'int' object to str implicitly

zeffii commented 9 years ago

cool. that 'll be my fault then. will get on it later!

zeffii commented 9 years ago

I was under the impression that the addon didn't work very well importing a few pdb files into one scene, and i've never really tried it since it first failed. The error message points to a trivial problem, but who knows..what kind of logic is behind it. Still not looked very deep into it.

MonZop commented 9 years ago

I am now out of Pisa, until thursday, but as far as I recall importing of several pdb in same scene did work. Will check when I get back

zeffii commented 9 years ago

localized the problem, will probably branch to fix as i'm not sure how deep it goes, fixing problems is sometimes like this: https://www.youtube.com/watch?v=D0n8N98mpes

zeffii commented 9 years ago

I think it's fixed now: https://github.com/MonZop/BioBlender/tree/BB2_Redux_s4_VIEW_pdbID

it is also broken in master: https://github.com/MonZop/BioBlender/blob/master/BioBlender2.py#L311-L318 , in python you can't add an integer to a string (it's bad practice in all languages but some allow it...)

MonZop commented 9 years ago

Great! (i do feel a little liket the evil soul that keep shooting moles out of the holes. But I hope I will run out of them at some point). And here's the next: Every time a new pdb is imported, BB seems to introduce a new Empty and a new Camera (and possibily also lights), all of them apparently on the same spot. I think this should not be.

Do you prefere moles one by one, or all together?

zeffii commented 9 years ago

all at once, it doesn't matter, but realize that the areas i haven't checked off on the list are "known unknowns", dealing with the checked ones first makes the most sense.

The camera and lights is an issue which I agree completely is not handled well at all. But the Empty, I am not sure I understand, isn't it useful to have a new empty per pdb so you can drag them around? (it's not my code so haven't adjusted it)

MonZop commented 9 years ago

In my hands the empty is not connected to the protein file. To move the protein I select it form the outliner, with its name

zeffii commented 9 years ago

oooh! i see, it creates an Extra empty. To be perfectly honest I am not sure what that's for, but it has keyframes associated with it.. OK good, I will need to investigage

MonZop commented 9 years ago

I can see no difference if i delete them

zeffii commented 9 years ago

I'm tempted to modify the EmptySet.blend and instead add the camera programatically, I think the Empty is part of the EmptySet.blend because it was considered convenient (?) to import camera+empty and duplicate those whenever needed. Both could easily be scripted, unless there's some secondary reason which I can't see.

Not sure when this will happen, my time and attention-span are limited at the moment.

MonZop commented 9 years ago

I don't see any reason for adding more cameras and empties for every imported PDB. I suspect it was just a sloppy way of addressing the request of importing several pdb files in the same scene.

zeffii commented 9 years ago

agreed, i'll tackle it next before proceeding further. This involves a scene variable which used to be a global variable, but I haven't interacted much with it yet. So I think at some point (before my time) the code was like

if (bootstrap == -1):
      file_append('EmptySet.blend')
      boostrap = 2   # never do this again

needs to become

if (bpy.context.scene.bb25_bootstrap == -1):
      file_append('EmptySet.blend')
      bpy.context.scene.bb25_bootstrap = 2   # never do this again

but it still imports the Empty and BioCamera once... as mentioned, the Empty is duplicated instead of generated fresh using.

new_empty = bpy.data.objects.new("some_empty", None)
zeffii commented 9 years ago

BB2_Redux_s5_BootstrapFixes

View panel has not entirely reached parity yet, but will fix bootstrapping first (multi pdb import)

I won't be able to avoid the redundant Empty for now, but have added the right code back to prevent reimport of EmptySet.blend

zeffii commented 9 years ago

some more unhandled globals:

# gui_panel_pdb_import.py
global chainCache
global curFrame  <-- will be scene variable

# gui_panel_view.py
global tag
global currentActiveObj
global oldActiveObj
MonZop commented 9 years ago

I wish I could be of more help, but the only comment I can provide is that the CurFrame might be used at another point, in the Export function when , after calculating the movement, BB can export the postion of all atoms in new pdb files, or can render many frames to produce a movie of the movieng protein

zeffii commented 9 years ago

Most things that were previously a global have been converted to scene variables, I had to do this for splitting up the file into manageable parts. In the end there should be no difference to the working code. curFrame will be OK as a scene variable (rather than global) chainCache is used in two functions but doesn't appear to depend on the content of the other functions version of chainCache so it doesn't even need to be a global..

zeffii commented 9 years ago

In blender 2.75 the new dependency graph is available to test by starting blender with blender --enable-new-depsgraph from command line. Obviously the redux isn't quite far enough to be able to really test that, but it may already have positive effect on the import of larger pdb .

zeffii commented 9 years ago

btw: It appears with the pdb import code almost unchanged, that the new dependency graph imports a little slower at the moment, I will ask sergey (a lead blender foundation coder) to look at the pdb import routine which handles the rigid body setup to see if there's anything obvious that can be done. If not then there are limited options left.. Maybe this should be discussed in a separate issue, at a later date.

Next I'll finish the bits of the view panel which still use globals, then move onto the next panel.

zeffii commented 9 years ago

I think perhaps EP panel is next.. all it does now is 'not display errors' , but is probably full of loose ends that need to be reattached.. no ETA

MonZop commented 9 years ago

Great, should I suse Blender 2.75 to test it, or 2.74 is OK?

zeffii commented 9 years ago

no need to test the new depsgraph yet, but always update to the latest when you have spare time. https://builder.blender.org/download/

MonZop commented 9 years ago

Just to be sure: the redux imports a PDB and builds the molecule with atoms and bonds, right? Is it possible to exclude the bonds if, for example, there is a molecule that is not a protein (such as the Heme part in hemoglobin, or lipid molelcues)? If I try to import a lipid I get an error, all atoms are imported at the origin

zeffii commented 9 years ago

are you testing on one of the included pdb?

zeffii commented 9 years ago

just to be clear, the import should be feature identical in the Redux as the current Master, but if they are different provide a link to the pdb so I can check the errors (no guarantee that i'll know how to fix them..)