Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve m-number adjoint bugs introduced by 1855 #2194

Merged
merged 5 commits into from
Sep 22, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
update change m number too
smartalecH committed Aug 12, 2022
commit 2dff39ef02a81431d36e566725aa8e04f12327fc
4 changes: 2 additions & 2 deletions python/simulation.py
Original file line number Diff line number Diff line change
@@ -4284,7 +4284,7 @@ def change_m_number(self, m):
Change the `m` number (the angular ϕ dependence).
"""
self.m = m

if self.fields:
needs_complex_fields = not (
not self.m or self.m == 0
@@ -4296,7 +4296,7 @@ def change_m_number(self, m):
self.init_sim()
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be better to just add a boolean argument to fields::use_real_fields so that we can switch back to real fields without initializing. And then this could be called by fields::change_m_number for m ≠ 1

else:
if self.m is not None:
self.fields.m = self.m
self.fields.change_m_number(m)

def change_sources(self, new_sources):
"""
11 changes: 11 additions & 0 deletions src/fields.cpp
Original file line number Diff line number Diff line change
@@ -793,4 +793,15 @@ bool operator==(const comms_key &lhs, const comms_key &rhs) {
return (lhs.ft == rhs.ft) && (lhs.phase == rhs.phase) && (lhs.pair == rhs.pair);
}

void fields::change_m_number(double new_m){
m = new_m;
for (int i = 0; i < num_chunks; i++) {
chunks[i]->change_m_number(new_m);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

At the very least, this needs to check that new_m is consistent with whether we have real fields and abort if not. Better yet, have it call if (new_m != 0) use_real_fields(false); as noted above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What do you envision use_real_fields(false) does in the case that the current simulation does have real fields (e.g. suppose originally m=0 but we are changing it to m=1). Wouldn't we need to reinitialize everything?

Or are you suggesting we refactor the initialization of the fields to use_real_fields(), such that

  • use_real_fields(true) deletes the extra array (the current behavior of use_real_fields())
  • use_real_fields(false) reallocates the fields if needed
  • does nothing if the simulation state is consistent

As a first pass, it might be easiest to leave use_real_fields as is and simply ensure there's a proper check in place for change_m_number() (like you suggest).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My latest commit adds a check to make sure that the fields are consistent. If the user requests complex fields when the current setup is using real fields, I just abort.

This isn't a problem for the adjoint code. We have a similar check in the python routine. But rather than aborting, we just reinitialize.

}

void fields_chunk::change_m_number(double new_m) {
m = new_m;
}

} // namespace meep
2 changes: 2 additions & 0 deletions src/meep.hpp
Original file line number Diff line number Diff line change
@@ -1546,6 +1546,7 @@ class fields_chunk {
void remove_sources();
void remove_susceptibilities(bool shared_chunks);
void zero_fields();
void change_m_number(double new_m);

// update_eh.cpp
bool needs_W_prev(component c) const;
@@ -1751,6 +1752,7 @@ class fields {
void remove_fluxes();
void reset();
void log(const char *prefix = "");
void change_m_number(double new_m);

// time.cpp
std::vector<double> time_spent_on(time_sink sink);