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.

Sign up for GitHub

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

BUG fix galsim random seeding again #28

Merged
merged 5 commits into from
Mar 27, 2024
Merged

BUG fix galsim random seeding again #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7ee3329
BUG fix galsim random seeding again
beckermr Mar 26, 2024
2d86391
Update des_tile.py
beckermr Mar 26, 2024
eb8cbd6
Update des_tile.py
beckermr Mar 26, 2024
c598977
ENH allow for missing truth data
beckermr Mar 27, 2024
0154c3e
ENH more carveouts for missing truth data
beckermr Mar 27, 2024
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
4 changes: 4 additions & 0 deletions montara/des_tile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,10 @@ def setup(self, config, base, file_num, logger):
if not isinstance(rs, list):
first = galsim.config.ParseValue(
base['image'], 'random_seed', base, int)[0]

# launder through the RNG to randomize
first = galsim.BaseDeviate(first).raw()
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain how this fixes the seeding bug?

Copy link

@rmjarvis rmjarvis Mar 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Sure.
GalSim uses a different random number generator for each object being rendered. This is so the stamp rendering can be parallelized over multiple processes deterministically. The random seed for each object is sequential based on the initial random seed (first here).
The problem that Sid was running into was that he was seeding different exposures with not very different random seeds. They were chosen from a set of integers that were fairly small. <32,000 maybe? I'm not sure exactly. But not nearly enough to make the sequences of seed for each exposure be disjoint from the sequences for any other exposure.
This fix (and the similar one in GalSim here) updates the given random seed to map onto some other value in [1,2^64). This essentially guarantees that the sequence of seeds that are used for each object in the exposure will be unique relative to any other exposure. Or at least the probability that they aren't is down at 1/2^64, rather than 1/2^15.

Copy link
Contributor

@esheldon esheldon Mar 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This sounds reasonable, but I think there might be something else going on in addition to whatever this is fixing.

The problem was way more severe than 1/2^15. Essentially the exact series of rotation values were used for objects in all simulated tiles. There seem to be a small fraction that mismatched between the lists, which is odd in itself.

For example there were 31281722 entries in the catalog I had from a large set of simulated tiles, but only 93233 unique rotation values. And when matt looked at the catalogs, most of them were in the same sequence, in order, for different images.

This is almost like the same initial seed is being used for each run of the code (although the claim is that this is not true). If something like that is going on, I think the above would not fix it, because this first = galsim.BaseDeviate(first).raw() would be going from the same initial state.

Copy link

Choose a reason for hiding this comment

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

I'm not definitively claiming that this is the only error. But it is a bug that we fixed in GalSim two years ago, which wasn't ported over to montara. It's possible there is something else wrong as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I think this is likely the bug. The truth files record every time an object is drawn across all exposures and bands. The fact that the rotation values are only unique for a subset is the correct result.

Instead the issue is that the unique values end up repeated across different tiles. This happens because of the small seeds as Mike points out.

Copy link
Contributor

Choose a reason for hiding this comment

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

My feeling is there is another issue as well, but we should go forward with this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep. My plan is to run some code at BNL to see what this fixes.

Copy link
Contributor

@esheldon esheldon Mar 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

One of the things that didn't make sense to me has been cleared up: I didn't realize the same object was in the truth catalog Nepoch times, which resulted in more duplicate rotation values than I would have predicted

Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering about that, but hadn't thought about nepochs (only bands). Glad that is worked out!


base['image']['random_seed'] = []
# The first one is the original random_seed specification,
# used for noise, since that should be different for each band,
Expand Down
121 changes: 62 additions & 59 deletions montara/eastlake_step.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,70 +370,73 @@ def _write_truth(self, fnames, tilename, base_dir, stash, bands):
else:
self.logger.warning("skipped zero-length truth file %r", fname)

if len(data) == 0:
if len(data) == 0 and self.config["output"].get("n_se_test", None) is None:
raise RuntimeError(
"No objects drawn for tile %s when using a grid!" % tilename
)

data = np.concatenate(data)
data = np.sort(data, order=["id", "band"])
if len(data) > 0:
data = np.concatenate(data)
data = np.sort(data, order=["id", "band"])

# we'll stash this for later
truth_filename = os.path.join(
base_dir,
"truth_files",
"%s-truthfile.fits" % tilename,
)
safe_mkdir(os.path.dirname(truth_filename))
self.logger.error(
"writing truth data to %s" % truth_filename)
fitsio.write(truth_filename, data, clobber=True)
stash.set_filepaths("truth_file",
truth_filename,
tilename)
for fname in fnames:
safe_rm(fname)

# now combine by band to make true positions files
uids, uinds = np.unique(data["id"], return_index=True)
n_pos_data = len(uids)
_pos_data = np.zeros(
n_pos_data,
dtype=[
('ra', 'f8'), ('dec', 'f8'),
('x', 'f8'), ('y', 'f8'),
('id', 'i8'),
] + [(f"mag_{b}", "f8") for b in bands],
)
_pos_data['id'] = data['id'][uinds]
_pos_data['ra'] = data['ra'][uinds]
_pos_data['dec'] = data['dec'][uinds]
_pos_data['x'] = data['x_coadd'][uinds]
_pos_data['y'] = data['y_coadd'][uinds]
_pos_data = np.sort(_pos_data, order="id")

for band in bands:
mskb = data["band"] == band
assert np.any(mskb)
bdata = data[mskb]
inds = np.searchsorted(_pos_data["id"], bdata["id"])
assert np.array_equal(_pos_data["id"][inds], bdata["id"])
_pos_data[f"mag_{band}"][:] = np.nan
_pos_data[f"mag_{band}"][inds] = bdata["mag"]

# we'll stash this for later
truepos_filename = os.path.join(
base_dir,
"true_positions",
"%s-truepositions.fits" % tilename,
)
safe_mkdir(os.path.dirname(truepos_filename))
self.logger.error(
"writing true position data to %s" % truepos_filename)
fitsio.write(truepos_filename, _pos_data, clobber=True)
stash.set_filepaths("truepositions_file",
truepos_filename,
tilename)
# we'll stash this for later
truth_filename = os.path.join(
base_dir,
"truth_files",
"%s-truthfile.fits" % tilename,
)
safe_mkdir(os.path.dirname(truth_filename))
self.logger.error(
"writing truth data to %s" % truth_filename)
fitsio.write(truth_filename, data, clobber=True)
stash.set_filepaths("truth_file",
truth_filename,
tilename)
for fname in fnames:
safe_rm(fname)

# now combine by band to make true positions files
uids, uinds = np.unique(data["id"], return_index=True)
n_pos_data = len(uids)
_pos_data = np.zeros(
n_pos_data,
dtype=[
('ra', 'f8'), ('dec', 'f8'),
('x', 'f8'), ('y', 'f8'),
('id', 'i8'),
] + [(f"mag_{b}", "f8") for b in bands],
)
_pos_data['id'] = data['id'][uinds]
_pos_data['ra'] = data['ra'][uinds]
_pos_data['dec'] = data['dec'][uinds]
_pos_data['x'] = data['x_coadd'][uinds]
_pos_data['y'] = data['y_coadd'][uinds]
_pos_data = np.sort(_pos_data, order="id")

for band in bands:
mskb = data["band"] == band
if self.config["output"].get("n_se_test", None) is None:
assert np.any(mskb)
if np.any(mskb):
bdata = data[mskb]
inds = np.searchsorted(_pos_data["id"], bdata["id"])
assert np.array_equal(_pos_data["id"][inds], bdata["id"])
_pos_data[f"mag_{band}"][:] = np.nan
_pos_data[f"mag_{band}"][inds] = bdata["mag"]

# we'll stash this for later
truepos_filename = os.path.join(
base_dir,
"true_positions",
"%s-truepositions.fits" % tilename,
)
safe_mkdir(os.path.dirname(truepos_filename))
self.logger.error(
"writing true position data to %s" % truepos_filename)
fitsio.write(truepos_filename, _pos_data, clobber=True)
stash.set_filepaths("truepositions_file",
truepos_filename,
tilename)

@classmethod
def from_config_file(cls, config_file, logger=None):
Expand Down
Loading