Nikea / VTTools

Library that has utility tools for VisTrails
Other
1 stars 8 forks source link

Fixed outport bug and added tomography #41

Closed celiafish closed 9 years ago

tacaswell commented 9 years ago

This needs to be rebased onto current master as there is a conflict that can not be auto-merged.

celiafish commented 9 years ago

Let me try to fix the conflict.

tacaswell commented 9 years ago

I think we either need to not use their dataset object and directly use the functions that do the work (my preference) and pass around ndarrays of the data, construct a new data set object for each function call and pass around the real data (annoying, but second place), or document these as what ever type of objects they really are and either expose that in VT as a module type or just use the 'variable' type (my least favorite).

Having a single mutable object that you make sequential calls on is very much against the view of the world that vistrails has (it is, at it's core, a functional language).

One of the design principles of skit-xray is to only use 'base' (core python + numpy) types as inputs/outputs of user-exposed functions.

celiafish commented 9 years ago

In that sense, vistrails is unique being a functional language in the OO world.... I agree with that design principle of skit-xray about using only base types. So how about this: 1. let me first put variable type instead of array_like, and demo it to Wah-Keat and others (I need his opinion about the functional benefits i.e. is it what he wants, if he wants more functions, requirements, etc.). 2. modify the code and pass around just ndarrays. This strategy should make sure the tomo package is on the right track and also it is coded in the right way.

tacaswell commented 9 years ago

From talking to Doga in April, I am pretty sure that there are clean functions underneath, the XTomoDataset objects mostly just does marshaling.

I think I will back the default value changes off of the wrap_extension branch (as it is broken for reasons I don't understand) so we can get that merged, which will make it easier to deal with arbitrary class mappings in VT.

celiafish commented 9 years ago

It really depends on how much time to spend on digging out their data structure and understand their code. They did marshaling to make it more OO look. So by passing the XTomoDataset object, we won't have huge number of parameters associated with each function call. It is less frustrating in users perspective though. I'd say, for plan A, let's treat our usage of tomopy as application level. This way it gains more time for us to focus on the usability of these functions. Once this is OK, let's go for plan B, which we care as development team, but the beamline scientists don't. Let's talk in details when we meet next time.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-1.26%) when pulling 55f157cec0c3a589abf2532d9e90aa46f8983e6d on celiafish:master into 35f569589673793bf98e0512c94fab1e35b90884 on Nikea:master.

celiafish commented 9 years ago

After I merged, got a new error when loading NSLS-II module in vistrails:

File "/home/wxu/NSLS2/VTTools/vttools/to_wrap/fitting.py", line 36, in <module>
    from skxray.fitting.api import (QuadraticModel, GaussianModel,
ImportError: cannot import name ExpressionModel

Any idea?

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-1.26%) when pulling 55f157cec0c3a589abf2532d9e90aa46f8983e6d on celiafish:master into 35f569589673793bf98e0512c94fab1e35b90884 on Nikea:master.

tacaswell commented 9 years ago

You need to update skxray and probably lmfit.

Aren't moving parts fun?

tacaswell commented 9 years ago

Both have conda packages built on my channel

celiafish commented 9 years ago

Oops... after updating skxray, the same error showed up the other day shows up again...

OSError: /home/wxu/anaconda/envs/vt_bare/bin/../lib/libm.so.6: version `GLIBC_2.15' not found (required by /usr/local/lib/libfftw3f.so.3)
tacaswell commented 9 years ago

sigh, try doing a conda remove system

tacaswell commented 9 years ago

and you did some sudo install stuff while installing fftw or are you using system fftw?

celiafish commented 9 years ago

Yes, I did sudo install. The fftw3f package seemed to be a separate one...

On Dec 11, 2014, at 6:58 PM, Thomas A Caswell notifications@github.com wrote:

and you did some sudo install stuff while installing fftw or are you using system fftw?

— Reply to this email directly or view it on GitHub.

tacaswell commented 9 years ago

We should avoid using anything with sudo a) to avoid clashing with the sysadmin on the beam line computers b) because not all users have sudo rights.

could you also try installing conda install glibc -c tacaswell? I re-compiled libc last night, I am curious if it fixes your problem. I suspect it won't as it will still be the wrong version of libm.... (but a newer one!)

celiafish commented 9 years ago

You can't imagine how much time I spent on installing tomopy.... Sure will try that.

On Dec 11, 2014, at 9:56 PM, Thomas A Caswell notifications@github.com wrote:

We should avoid using anything with sudo a) to avoid clashing with the sysadmin on the beam line computers b) because not all users have sudo rights.

could you also try installing conda install glibc -c tacaswell? I re-compiled libc last night, I am curious if it fixes your problem. I suspect it won't as it will still be the wrong version of libm.... (but a newer one!)

— Reply to this email directly or view it on GitHub.

tacaswell commented 9 years ago

We should also avoid inflicting that amount of pain on our users ;)

celiafish commented 9 years ago

Tried install glibc. Now have segmentation fault...

tacaswell commented 9 years ago

@celiafish Is this different than your other PR?

celiafish commented 9 years ago

This was an old PR. The useful one is fixing outport bug part. Other than that you can simply ignore this one.