WIP: Allow unequal DFT frequencies #1141

danielwboyce · 2020-03-02T23:52:50Z

~~Fixes #1070.~~ In progress of changing the relevant DFT classes' properties so that they have an array of frequencies, not freq_min or freq_max. The existing constructors will be updated so that DFT objects can be defined with freq_min, freq_max, and Nfreq, but new constructors will also be added that use only freqs and Nfreq.

…ions using dft. Need to hunt down any possible uses of them...

…per functions, etc. to header

smartalecH · 2020-03-03T02:22:51Z

Some swig advice:

When wrapping overloaded C constructors, Swig needs all of the parameters to be independently type-checked (see docs). This is good practice anyway... If you want to check a numpy array called freqs, you can do something like this:

%typecheck(SWIG_TYPECHECK_POINTER, fragment="NumPy_Fragments") double* freqs {
    $1 = is_array($input);
}

(all within meep.i of course)

You'll then have to define a custom typemap. You can do something like this:

%typemap(in, fragment="NumPy_Macros") double* freqs {
    $1 = (double *)array_data($input);
}

Make sure you do this for all the fields."flux" methods (i.e. add_dft_flux, etc.)

You'll have to go through simulation.py and update all of the flux, force, far_field, eigenmode, etc methods.

…per functions, etc.

…per functions, etc. to header

…per functions, etc.

… functions

…til swig comes into play

danielwboyce · 2020-03-04T01:39:08Z

As of now, the source code seems to be properly compiling (none of the constructors seem to have been broken). Now I'll need to update python/meep.i and scheme/meep.i to have support in the scripting languages, which should also make testing easier.

* Add TODOs for functions that need to be overloaded

* Wrapped add_* functions around add_*_uneven functions * Changed parameters of _add_* functions

…hat quantity df was.

danielwboyce · 2020-03-11T03:49:48Z

As of now the package compiles all the way through, but I'm sure I'm getting SWIG wrong. The version of python/meep.i that's in place has the minimal changes in place required to build the package with the changes to the C code and simulation.py. I notice that for some reason the overloaded constructors don't need me to define new typemaps or typechecks to compile; is it likely that the automatically-generated wrappers are correct?

While attempting to run the python/examples/mie_scattering.py tutorial example I'm getting the following error

Traceback (most recent call last):
  File "/home/tests/mie_scattering.testing_v.pass_frq_min_frq_max.py", line 51, in <module>
    sim.run(until_after_sources=10)
  File "/usr/local/lib/python3.6/site-packages/meep/simulation.py", line 2259, in run
    self._evaluate_dft_objects()
  File "/usr/local/lib/python3.6/site-packages/meep/simulation.py", line 1627, in _evaluate_dft_objects
    dft.swigobj = dft.func(*dft.args)
  File "/usr/local/lib/python3.6/site-packages/meep/simulation.py", line 1851, in _add_flux
    return self._add_fluxish_stuff(self.fields.add_dft_flux, freqs, nfreq, fluxes)
  File "/usr/local/lib/python3.6/site-packages/meep/simulation.py", line 1987, in _add_fluxish_stuff
    stuff = add_dft_stuff(vol_list, freqs, nfreq, *args)
  File "/usr/local/lib/python3.6/site-packages/meep/__init__.py", line 3959, in add_dft_flux
    return _meep.fields_add_dft_flux(self, *args)
NotImplementedError: Wrong number or type of arguments for overloaded function 'fields_add_dft_flux'.
  Possible C/C++ prototypes are:
    meep::fields::add_dft_flux(meep::volume_list const *,double *,int,bool)
    meep::fields::add_dft_flux(meep::volume_list const *,double *,int)
    meep::fields::add_dft_flux(meep::volume_list const *,double,double,int,bool)
    meep::fields::add_dft_flux(meep::volume_list const *,double,double,int)
    meep::fields::add_dft_flux(meep::direction,meep::volume const &,double *,int,bool)
    meep::fields::add_dft_flux(meep::direction,meep::volume const &,double *,int)
    meep::fields::add_dft_flux(meep::direction,meep::volume const &,double,double,int,bool)
    meep::fields::add_dft_flux(meep::direction,meep::volume const &,double,double,int)

Which seems to tell me that the add_dft_flux function is not playing nice with SWIG. Thing is there's no add_dft_flux function mentioned in python/meep.i! Was the wrapper getting generated automatically before? Will I need to make a typemap suite for it now? Which parameters will I need to check? All of them, or just the ones that differ between the overloaded versions of the function (double freq_min, double freq_max, double *freqs)?

…s_1070

smartalecH · 2020-03-11T16:42:32Z

The python/swig UI regarding DFT stuff is a little convoluted...largely because of the load balancing requirements. But luckily I think you only need some minor changes to get your code working.

First, let's remember that all the DFT functions using fluxes will call add_dft_flux through _add_fluxish_stuff:

meep/python/simulation.py

Lines 1820 to 1823 in 6c10e74

    
           def _add_flux(self, fcen, df, nfreq, fluxes): 
        
               if self.fields is None: 
        
                   self.init_sim() 
        
               return self._add_fluxish_stuff(self.fields.add_dft_flux, fcen, df, nfreq, fluxes)

You can see that _add_fluxish_stuff assumes one particular constructor form for the corresponding add_dft_stuff function (add_dft_flux in this case):

meep/python/simulation.py

Lines 1939 to 1955 in 6c10e74

    
           def _add_fluxish_stuff(self, add_dft_stuff, fcen, df, nfreq, stufflist, *args): 
        
               vol_list = None 
        
               for s in stufflist: 
        
                   v = Volume(center=s.center, size=s.size, dims=self.dimensions, 
        
                              is_cylindrical=self.is_cylindrical) 
        
                   d0 = s.direction 
        
                   d = self.fields.normal_direction(v.swigobj) if d0 < 0 else d0 
        
                   c = mp.direction_component(mp.Sx, d) 
        
                   v2 = Volume(center=s.center, size=s.size, dims=self.dimensions, 
        
                               is_cylindrical=self.is_cylindrical).swigobj 
        
                   vol_list = mp.make_volume_list(v2, c, s.weight, vol_list) 
        
               stuff = add_dft_stuff(vol_list, fcen - df / 2, fcen + df / 2, nfreq, *args) 
        
               vol_list.__swig_destroy__(vol_list) 
        
               return stuff

This is problematic for two reasons:

First, you need to generalize the python code so we can either use the original constructor (i.e. specify center, width, and the number of freqs) or pass in an array (like you've already implemented on your branch).
You need to explicitly typecheck and typemap every argument of all the possible constructors. Even if it compiles, that doesn't mean it works! The typechecking mechanism is how swig determines which constructor to use. It assigns a weight to each paramter based on it's type and maps the cumulative weight to a table. You can see more about that in the docs I posted above.

Feel free to check out PR #1095 for an example. I "overloaded" add_dft_fields (in reality I just added a default argument, but SWIG treats this as an overloaded constructor). I had to typecheck the remaining parameters. I was getting the same error as you until I realized this small nuance.

Looking forward to this feature!

ChristopherHogan · 2020-03-12T01:26:07Z

I think what you want is to apply the predefined numpy typemaps. If you have double *freqs, int Nfreqs pairs of arguments for the new functions and they're all named consistently, all you need in python/meep.i is

%apply (double *IN_ARRAY1, int DIM1) {(double *freqs, int Nfreqs)};

ChristopherHogan · 2020-03-12T01:41:59Z

https://docs.scipy.org/doc/numpy-1.13.0/reference/swig.interface-file.html

ChristopherHogan · 2020-03-12T02:32:45Z

python/simulation.py

        self.dft_objects.append(flux)
        return flux

-    def _add_flux(self, fcen, df, nfreq, fluxes):
+    def add_flux(self, fcen, df, nfreq, *fluxes):
+        freqs = np.linspace(fcen - df / 2, fcen + df / 2, nfreq).tolist()


You can remove the tolist() call since the typemaps are expecting a numpy array.

ChristopherHogan · 2020-03-12T02:33:21Z

python/meep.i

+    $1 = is_array($input);
+}
+
+%typemap(in, fragment="NumPy_Macros") double* freqs {


This typemap isn't needed. It's covered by the %apply below.

ChristopherHogan · 2020-03-12T02:33:41Z

python/meep.i

+    $1 = PyInteger_Check($input);
+}
+
+%typemap(in) int Nfreqs {


This typemap isn't needed. It's covered by the %apply below.

danielwboyce · 2020-03-12T05:07:33Z

Removing tolist where extraneous and the extra typechecks doesn't seem to have fixed the problem.

stevengj · 2020-03-17T17:08:22Z

src/dft_ldos.cpp

+    Nomega = Nfreq;
+    double om[Nomega];
+    for (int i = 0; i < Nfreq; i++) { om[i] = freqs[i] * 2 * pi; }
+    omegas = om;


This won't work. om will no longer exist once this scope exits. You need to allocate on the heap with e.g. new

stevengj · 2020-03-17T17:09:37Z

src/dft_ldos.cpp

-    domega = 0;
-    Nomega = 1;
+// TODO: overload dft_ldos constructor
+dft_ldos::dft_ldos(double *freqs, int Nfreq) {


If the object makes a copy of freqs, then this should be declared as const double *freqs.

stevengj · 2020-03-17T17:10:18Z

src/dft.cpp

+                           bool use_centered_grid, int vc) {
+  if (Nfreq < 1) abort("Nfreq must be at least 1");
+  double dfreq = (freq_max - freq_min) / (Nfreq - 1);
+  double freqs[Nfreq];


Same problem: this does not persist.

stevengj · 2020-03-17T17:11:49Z

src/meep.hpp

  int Nomega;
+  double *omegas;


might want to use std::vector<double> here.

stevengj · 2020-03-17T17:14:44Z

Before you worry about getting the Python interface to work, I would to get the C++ tests to work, e.g. tests/bend-flux-ll.cpp and tests/dft-fields.cpp.

stevengj · 2020-03-17T17:16:02Z

In fact, let's separate the Python stuff into a separate PR. Just try to get the C++ plumbing in place in this PR.

danielwboyce · 2020-03-17T17:18:28Z

Let's also add a C++ test for the new plumbing in this PR.

oskooi · 2020-03-23T02:08:11Z

Note that this PR is no longer necessary as it has been replaced by #1154.

danielwboyce added 7 commits March 1, 2020 16:22

added some TODOs, need more

ca82d45

Should be all TODOs for src/meep.hpp

7a21f0e

Should have added TODOs for all relevant function/class/struct defint…

0ba7651

…ions using dft. Need to hunt down any possible uses of them...

Changed freq_min/freq_max/dfreq to freqs

354b460

Changed freq_min/freq_max/dfreq to freqs

612eb98

greater detail in TODOs; add overloaded versions of constructors, hel…

286e22f

…per functions, etc. to header

greater detail in TODOs; add overloaded versions of constructors, hel…

ea7ba08

…per functions, etc. to header

danielwboyce added 8 commits March 2, 2020 20:30

typo

92895cf

details on TODOs, added some overloaded constructors

4cfc13b

greater detail in TODOs; add overloaded versions of constructors, hel…

157638f

…per functions, etc.

greater detail in TODOs; add overloaded versions of constructors, hel…

c24fa0a

…per functions, etc. to header

greater detail in TODOs; add overloaded versions of constructors, hel…

d89d799

…per functions, etc.

greater detail in TODOs; add overloaded versions of constructors, hel…

1698f2d

…per functions, etc.

Fixed how freqs is constructed and added freqs deletion to the remove…

b4f0926

… functions

changed freqs definitions in classes to be just pointers. compiles un…

a429fd8

…til swig comes into play

danielwboyce added 7 commits March 8, 2020 23:10

* Change properties of DftObj class.

67c3205

* Add TODOs for functions that need to be overloaded

* Created add_*_uneven functions

082247d

* Wrapped add_* functions around add_*_uneven functions * Changed parameters of _add_* functions

Make sure freqs is sorted

9f986bf

Removed (nfreq - 1) term from linspace functions; had misunderstood w…

1f972bf

…hat quantity df was.

Replacement of get_omega_min and get_domega with get_omegas

b10366c

Alterations for dft_ldos in python/meep.i

e075a34

Change because of differing number of arguments

d7c8a7d

Merge branch 'master' of https://github.com/danielwboyce/meep into is…

8212547

…s_1070

danielwboyce added 2 commits March 11, 2020 18:44

Additional attempt NanoComp#1 at typemaps in SWIG

bee4c0b

cleanup

c781850

this is certainly wrong

4dd0683

ChristopherHogan reviewed Mar 12, 2020

View reviewed changes

danielwboyce added 2 commits March 11, 2020 21:22

Remove extra tolist()

df25758

python/meep.i still doesn't seem to be working

db61366

stevengj reviewed Mar 17, 2020

View reviewed changes

stevengj mentioned this pull request Mar 20, 2020

arbitrary frequency grid for DFT functions in C interface #1154

Merged

danielwboyce added 6 commits March 22, 2020 17:44

Merge branch 'master' of https://github.com/NanoComp/meep into iss_1070

89ffda4

Revert python/meep.i and python/simulation.py to master state

27f4536

Replace freqs arrays with std::vectors

772a906

Replace freqs arrays with std::vectors

0e54974

Replace freqs arrays with std::vectors

b6c2317

Replace freqs arrays with std::vectors

ab83da1

danielwboyce added 3 commits March 22, 2020 20:09

Replace freqs arrays with std::vectors

12c40aa

Replace freqs arrays with std::vectors

fe3c650

Replace freqs arrays with std::vectors

9fdc3c9

stevengj closed this in #1154 Mar 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Allow unequal DFT frequencies #1141

WIP: Allow unequal DFT frequencies #1141

danielwboyce commented Mar 2, 2020 •

edited

Loading

smartalecH commented Mar 3, 2020

danielwboyce commented Mar 4, 2020

danielwboyce commented Mar 11, 2020

smartalecH commented Mar 11, 2020

ChristopherHogan commented Mar 12, 2020 •

edited

Loading

ChristopherHogan commented Mar 12, 2020

ChristopherHogan Mar 12, 2020

ChristopherHogan Mar 12, 2020

ChristopherHogan Mar 12, 2020

danielwboyce commented Mar 12, 2020

stevengj Mar 17, 2020

stevengj Mar 17, 2020

stevengj Mar 17, 2020

stevengj Mar 17, 2020

stevengj Mar 17, 2020

stevengj commented Mar 17, 2020

stevengj commented Mar 17, 2020

danielwboyce commented Mar 17, 2020

oskooi commented Mar 23, 2020

WIP: Allow unequal DFT frequencies #1141

WIP: Allow unequal DFT frequencies #1141

Conversation

danielwboyce commented Mar 2, 2020 • edited Loading

smartalecH commented Mar 3, 2020

danielwboyce commented Mar 4, 2020

danielwboyce commented Mar 11, 2020

smartalecH commented Mar 11, 2020

ChristopherHogan commented Mar 12, 2020 • edited Loading

ChristopherHogan commented Mar 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielwboyce commented Mar 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevengj commented Mar 17, 2020

stevengj commented Mar 17, 2020

danielwboyce commented Mar 17, 2020

oskooi commented Mar 23, 2020

danielwboyce commented Mar 2, 2020 •

edited

Loading

ChristopherHogan commented Mar 12, 2020 •

edited

Loading