Skip to content

Include eps_parameters for plot3D#3205

Open
drinwater wants to merge 8 commits intoNanoComp:masterfrom
drinwater:plot3deps
Open

Include eps_parameters for plot3D#3205
drinwater wants to merge 8 commits intoNanoComp:masterfrom
drinwater:plot3deps

Conversation

@drinwater
Copy link
Copy Markdown

Currently plot3D cannot plot materials with frequency-dependent epsilon. This PR fixes that by including eps_parameters kwarg in plot3D

Comment thread python/simulation.py Outdated
stevengj
stevengj previously approved these changes Apr 24, 2026
Comment thread python/visualization.py Outdated
Comment thread python/visualization.py Outdated
Comment thread python/visualization.py Outdated
eps_data = np.round(np.real(sim.get_epsilon_grid(xtics, ytics, ztics)), 2)

unique = np.unique(np.abs(eps_data)).tolist()
eps_data = np.round(np.real(sim.get_epsilon_grid(xtics, ytics, ztics, eps_frequency)), 2)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why we are rounding here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous versions of vispy used to throw an error when not rounded for some reason back when plot3D was first added. I will remove this now since that issue seems to be fixed.

Comment thread python/visualization.py Outdated
@stevengj stevengj dismissed their stale review April 24, 2026 16:26

see comment

drinwater and others added 2 commits April 24, 2026 12:31
Comment thread python/visualization.py
unique = np.unique(eps_data).tolist()

# Remove background material
unique.remove(np.round(np.abs(np.asarray(sim.default_material.epsilon_diag)), 2)[0])
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smartalecH, do you remember why round was being used here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants