search

XENON1T / pax

The XENON1T raw data processor [deprecated]
BSD 3-Clause "New" or "Revised" License
16 stars 15 forks source link

tensorflow based nn of SR0 #651

Closed weiyuehuan closed 6 years ago

weiyuehuan commented 6 years ago

These two files are for SR0 Tensorflow NN.

SR1 Tensorflow based NN can be found: https://github.com/XENON1T/pax/pull/650

tunnell commented 6 years ago

Can you please save your models using model.save('blah.hdf5')? This puts everything in one file rather than two. It reduces the inevitable mess in pax. See the other hax pull request.

feigaodm commented 6 years ago

@tunnell There is one advantage of saving them into pieces because one can see the structure/version of the network in the json file, if we just do a h5 file we will not be able to see it. I think it's fine, unless you tell me that it introduce some problems in usage?

tunnell commented 6 years ago

Why do you think you can't see version the other way? You can do the same thing with both. Even model.summary(). The two big downsides are that it means files can get out of sync and doubles the size of these files that are being stored in the repository. It's just twice as clean and avoids a potential problem, why wouldn't you do it?

Moreover, it's just the right way to save models, right? Look at the Checkpointer callbacks for example.

pdeperio commented 6 years ago

Also, not sure how to include the bad PMT list, other than in the .json as in https://github.com/XENON1T/pax/pull/651. So merging for now to get a production going.

tunnell commented 6 years ago

@feigaodm What's the point of code review if they are completely ignored and you merge anyways? These were legitimate concerns about making a mess that I'd be responsible for cleaning up in the future.

pdeperio commented 6 years ago

not completely ignored, just on a deadline. we're aware of the mess and will help cleanup as soon as we know how.