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

Meep geom #56

Merged
merged 11 commits into from
Jun 17, 2017
Merged

Meep geom #56

merged 11 commits into from
Jun 17, 2017

Conversation

HomerReid
Copy link
Contributor

Replaces previous meepgeom PR.

Adds new folder libmeep_geom to top-level source tree with three working tests (pw-source-ll.cpp, bend-flux-ll.cpp, ring-ll.cpp`).

@oskooi @ChristopherHogan

@HomerReid
Copy link
Contributor Author

HomerReid commented Jun 16, 2017

In the most recent commit, I have

  • renamed meep_geom to meepgeom

  • removed bend-flux-ll from the test suite, as it was failing in the travis build. I wasn't able to debug exactly why this is, but I'm guessing it's because the code tries to write and then read back a largish HDF5 file which may not work in the travis build space. For now the test suite contains two tests which are passing in travis.

material_data vacuum_material_data;
bool materials_initialized = false;

void initialize_materials()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this need to be a function? Can't you just use a static initializer const material_type vacuum = {....}?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this is what was preventing the build from working on Travis. Previously I had all the initializations done the way you suggested, but these static structure initializations require the -std=c++11 compiler flag, which is not supported by the compiler versions used by Travis.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Static structure initializations can be done even in 1990 ANSI C. You just can't use named fields. i.e.

const medium_struct vacuum_medium = {{1,1,1},{0,0,0},{1,1,1},{0,0,0}};
const material_data vacuum_material = {MEDIUM,NULL,NULL,&vacuum_medium};
const material_type vacuum = {(void*) &vacuum_material};

(Note that the unspecified extra fields in vacuum_medium are initialized to 0, per the C/C++ standard.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, in the latest commit (a1117e6) I've eliminated the initialize_materials() function in favor of static structure initializers.

material_type vacuum { (void *)&vacuum_material_data };
material_type air = vacuum;
vacuum.data=(void *)&vacuum_material_data;
air = vacuum;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wouldn't bother with the the air synonym in the C++ interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I removed it.

@stevengj stevengj merged commit 8319eae into NanoComp:master Jun 17, 2017
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