jumasy
commented
6 years ago On Sep 11, 2017, at 8:41 AM, Tiago M. D. Pereira notifications@github.com wrote:
@tiagopereira commented on this pull request.
In helita/sim/bifrost.py https://github.com/ITA-Solar/helita/pull/4#discussion_r138108797:
else:raise ValueError(('_get_composite_var: do not know (yet) how to'
- 'get composite variable %s.' % var))
- 'get composite variable %s. Note that'
- 'simple_var available variables are: %s' % (var,repr(self.simple_vars))))
- def do_mesh(self, x=None, y=None, z=None, nx=None, ny=None, nz=None, This function is not generic. It has values hardcoded. It does not belong here!
I asked where do you want to have the do_mesh (BTW, this do_mesh is 3 times shorter than the idl version!). I want to add code that allows me to create snapshots, mesh and tables (also for ebysus that I have already partially done).
In helita/sim/cstagger.pyx https://github.com/ITA-Solar/helita/pull/4#discussion_r138109157:
-ctypedef fused FLOAT_t: This should not be removed. Now cstagger.pyx uses fused types.
Ups, something I did wrong here. I though I had this as you had. I wonder if I deleted some version that I had and I shouldn’t… Do you want me to go through this?
In helita/sim/cstagger.pyx https://github.com/ITA-Solar/helita/pull/4#discussion_r138109360:
- From init_stagger.c and init_stagger.pro
- Parameters
- mz - integer
- Number of z points
- z - 1-D ndarray, float
- z scale
- zdn - 1-D ndarray, float
- z scale derivative
- Returns
- None. Results saved in cstagger.zupc, cstagger.zdnc.
- ''' +def init_stagger(int mz, FLOAT_t dx, FLOAT_t dy, np.ndarray[FLOAT_t, ndim=1] z, np.ndarray[FLOAT_t, ndim=1] zdn, np.ndarray[FLOAT_t, ndim=1] dzup, np.ndarray[FLOAT_t, ndim=1] dzdn): Again, this is replacing previously updated code with an old version.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ITA-Solar/helita/pull/4#pullrequestreview-61862017, or mute the thread https://github.com/notifications/unsubscribe-auth/ANp4BDbMbuuFBXQj2yv591lSEDQmuX2cks5shVSlgaJpZM4PNbnQ.
I am not sure what is wrong there. Note that the version above is ready to do derivative and xdn and ydn etc.
J
tiagopereira
I am unable to merge this as is. It is based on an older version of the repository and there are several conflicts. I will have to go through the changes manually and put in the new parts in the current master.