Conversation
|
retest this please |
Rohit-Kakodkar
left a comment
Rohit-Kakodkar
left a comment
There was a problem hiding this comment.
LGTM. Pretty straightforward.
| * | ||
| */ | ||
| ///@{ | ||
| DEFINE_POINT_VALUE(mu0, 0) ///< Magnetic permeability @f$ \mu_0 @f$ |
There was a problem hiding this comment.
I think the only property you need here is inverse_magnetic_permeability when there is no attenuation. Since we only use that when computing acceleration. See https://github.com/SPECFEM/specfem2d/blob/11e23032568fce4b93b9491a4d1bb7829341b0fb/src/specfem2D/compute_forces_electromagnetic.F90#L181 . You'd also need the conductivity tensor when computing damping see : https://github.com/SPECFEM/specfem2d/blob/11e23032568fce4b93b9491a4d1bb7829341b0fb/src/specfem2D/compute_forces_electromagnetic_conduction.F90#L72
There was a problem hiding this comment.
Storing the rest of them here would increase computation time.
There was a problem hiding this comment.
There was a problem hiding this comment.
You're totally right, I cross checked with the paper, but not with the code.
Looks like we need both permittivity for the mass matrix and conductivity for the
cinductivity computation. But here it may make sense to just compute the when assigning the point properties?
There was a problem hiding this comment.
What do you mean assignment? It would make sense to store them within compute/properties as well, right?
There was a problem hiding this comment.
What do you mean assignment? It would make sense to store them within compute/properties as well, right?
There was a problem hiding this comment.
What do you mean assignment? It would make sense to store them within compute/properties as well, right?
… and medium/EM/materials
|
retest this please |
| * | ||
| */ | ||
| ///@{ | ||
| DEFINE_POINT_VALUE(mu0, 0) ///< Magnetic permeability @f$ \mu_0 @f$ |
There was a problem hiding this comment.
What do you mean assignment? It would make sense to store them within compute/properties as well, right?
| * | ||
| */ | ||
| ///@{ | ||
| DEFINE_POINT_VALUE(mu0, 0) ///< Magnetic permeability @f$ \mu_0 @f$ |
There was a problem hiding this comment.
What do you mean assignment? It would make sense to store them within compute/properties as well, right?
| * | ||
| */ | ||
| ///@{ | ||
| DEFINE_POINT_VALUE(mu0, 0) ///< Magnetic permeability @f$ \mu_0 @f$ |
There was a problem hiding this comment.
What do you mean assignment? It would make sense to store them within compute/properties as well, right?
| struct kernels<specfem::dimension::type::dim2, | ||
| specfem::element::medium_tag::electromagnetic_sv, | ||
| specfem::element::property_tag::isotropic, UseSIMD> | ||
| : public impl::point_data<10, UseSIMD> { |
|
Youre also missing a include header for |
Rohit-Kakodkar
left a comment
Rohit-Kakodkar
left a comment
There was a problem hiding this comment.
Are you implementing get_properties tests in the next PR?
Yes i need to get rid of 590 first now |
lsawade
changed the title
lsawade
added
the
new physics
3 participants
Description
include/point/properties.hppinclude/point/kernel.hppIssue Number
Closes #587
Closes #588
Checklist
Please make sure to check developer documentation on specfem docs.