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

pass binary-tree partition to structure constructor #1577

Merged
merged 2 commits into from
May 28, 2021

Conversation

oskooi
Copy link
Collaborator

@oskooi oskooi commented May 22, 2021

Fixes #1573.

Based on a suggestion in #1528 by @stevengj, this PR revamps the structure class constructor to accept a binary_partition argument which enables specifying the chunk volumes once when the user passes in a BinaryPartition object from the Python API. Correspondingly, the Python function load_chunk_layout has been removed since it is no longer necessary. The main changes involved the function split_by_cost in structure.cpp which now returns a binary_partition object rather than writing the chunk volumes into an array.

This PR enables creating the PML regions as separate chunks using the existing code structure::choose_chunkdivision. As a result, the example test described in #1573 now produces the correct chunk layout:

bp_chunk_layout

All the C++ tests are passing yet several of the Python tests are failing. This is due to a SWIG-related issue that needs to be resolved before this PR can be merged. An additional argument has been added to the C++/Python function create_structure_and_set_materials in the SWIG typemaps file meep.i in order to pass in the BinaryPartition pointer object (which is then passed to the structure class constructor). The problem is that trying to pass in a None (for a NULL pointer) for this argument produces an error:

======================================================================
ERROR: test_dft_energy (__main__.TestDftEnergy)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "dft_energy.py", line 29, in test_dft_energy
    sim.run(until_after_sources=100)
  File "/meep/python/meep/simulation.py", line 3637, in run
    self.init_sim()
  File "/meep/python/meep/simulation.py", line 1909, in init_sim
    self._init_structure(self.k_point)
  File "/meep/python/meep/simulation.py", line 1699, in _init_structure
    self.chunk_layout if isinstance(self.chunk_layout,mp.BinaryPartition) else None
SystemError: <built-in function create_structure_and_set_materials> returned NULL without setting an error

The issue is that SWIG is expecting a meep::binary_partition *my_bp type argument and does not accept NULL.

@oskooi oskooi changed the title WIP: pass binary-tree partition to structure constructor pass binary-tree partition to structure constructor May 25, 2021
@oskooi
Copy link
Collaborator Author

oskooi commented May 25, 2021

To pass a None value as the BinaryPartition object simply required removing the SWIG failure check for this particular case which had been previously added in #1528 to the typemaps file and is no longer necessary:

meep/python/meep.i

Lines 1457 to 1462 in ef8f6fc

%typemap(in) meep::binary_partition * {
$1 = py_bp_to_bp($input);
if(!$1) {
SWIG_fail;
}
}

All the tests are now passing and this PR is ready to be merged.

@stevengj stevengj merged commit e79addd into NanoComp:master May 28, 2021
@oskooi oskooi deleted the split_by_cost_revamp branch May 28, 2021 01:20
bencbartlett pushed a commit to bencbartlett/meep that referenced this pull request Sep 9, 2021
* pass binary-tree partition to structure constructor

* disable SWIG failure if user-supplied BinaryPartition object is None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

user-specified cell partition with boundary layers
2 participants