danieljfarrell / pvtrace

Optical ray tracing for luminescent materials and spectral converter photovoltaic devices
Other
97 stars 94 forks source link

add_ray_paths does not return identifiers #27

Closed nro-bot closed 4 years ago

nro-bot commented 4 years ago

Hi, Thanks for work on pvtrace! I think add_ray_paths has small bug. Specifies return but returns none.

            Returns
            -------
            identifier : str
                The string identifier used to add the line to the scene.

https://github.com/danieljfarrell/pvtrace/blob/e1459c85058bca1f53cda7e0e1177d6c0417fe5d/pvtrace/scene/renderer.py#L221

My pull request would return a list of ids:

def add_ray_path(self, rays: [Ray]) -> str:
        vis = self.vis
        if len(rays) < 2:
            raise AppError("Need at least two points to render a line.")
        ids = []
        for (start_ray, end_ray) in zip(rays[:-1], rays[1:]):
            nanometers = start_ray.wavelength
            start = start_ray.position
            end = end_ray.position
            colour = wavelength_to_hex_int(nanometers)
            identifier = self.add_line_segment(start, end, colour=colour)
            ids.append(identifier)
        return ids

Use case: interactively move a light source around, remove the old rays. I'm not at all sure this is the right way to do it.

def update_light_position(x,y,z):
    vis.remove_object(list_of_rays)
    (then create new rays)
danieljfarrell commented 4 years ago

Do you want to do real time ray tracing with pvtrace? For example, moving an object in the scene and update the position of a few rays.

This should be possible. However, it certainly was not something I was considering when designing the architecture. At the moment there is no notification mechanism which tells the view (renderer) that the model (scene) has updated.

So probably the best way to start is not but adding any new code. Instead, try writing a script which moves a light source and creates a new scene for each (do that in a for loop) then trace the whole scene but reuse the same renderer object (created outside the for loop).

nro-bot commented 4 years ago

Hi! Thanks for your response. I don't actually want to do real-time ray-tracing, I just wanted to figure out the x,y,z coordinates I want for my light source. I have a mesh file and was trying to locate the light relative to it, so I wanted to move the light around. Thanks for explaining rendered vs scene vs object. I will try your suggestion when I return to this problem next week.

I noticed I forgot to update the type to a List:

def add_ray_path(self, rays: [Ray]) -> List[str]:
        vis = self.vis
        if len(rays) < 2:
            raise AppError("Need at least two points to render a line.")
        ids = []
        for (start_ray, end_ray) in zip(rays[:-1], rays[1:]):
            nanometers = start_ray.wavelength
            start = start_ray.position
            end = end_ray.position
            colour = wavelength_to_hex_int(nanometers)
            identifier = self.add_line_segment(start, end, colour=colour)
            ids.append(identifier)
        return ids

========== The longer story i that I am trying to do light piping analysis, for a gelsight sensor, in order to help with # of LEDsand placement for balancing the R, G, B lights going into the edges of an acrylic tube (part of the tube surface also has a gel attached with semi-specular coating). I was using a TracePro trial but I think it is too complex/expensive for my needs, though I am coming around to its user interface. It's also possible I actually want to use something more like Blender.

Video of rays being traced after I used trial-and-error to find where to put my one LED.

(The video shows a separate issue which I might file an issue for, but I will debug first. It occurs after a small but variable number of rays are traced:

     69         if not np.any(np.absolute(distances) < EPS_ZERO):
---> 70            raise GeometryError('Point is not on surface.', {"point": surface_point, "geometry": self, "distances": distances, "threshold": EPS_ZERO})

===============

A light guide diagram: image

CAD model of tube + gel (with semispecular surface) + LEDs on top and bottom image

Tracepro result image

I think it's likely I'll end up having to move to Blender (in which I case I wouldn't file any more issues), but if I end up using pvtrace I will make sure to cite it. Thanks !

danieljfarrell commented 4 years ago

Good job, seems you are almost there. Great to see the pvtrace video. If you want, share you script with me so I can have a look. It could be something simple like the world node is clipping the mesh. If this happens the algorithms breaks down. But otherwise I would have to debug it to understand the error message.

danieljfarrell commented 4 years ago

Fixed in release pvtrace-2.1.0.

Going to close. Good luck with the ray-tracing. Please re-open if you need some help.

nro-bot commented 4 years ago

Thank you! Sorry, I've a bit of other work for a lab meeting, but will respond more after Wednesday.