fix_sso_w_sourcegen#28
Conversation
…tible. fix source generator problem
| sso = client.sso.get_sso_MPC_id(MPC_id=mpc_id)[0] | ||
|
|
||
| for _, row in tqdm( | ||
| rows[::10].iterrows(), desc=f"Ingesting {designation}", total=len(rows[:10]) |
| rows.iterrows(), desc=f"Ingesting {designation}", total=len(rows) | ||
| ): | ||
| flux = None | ||
| flux = 200 * u.mJy # Default flux if not provided |
There was a problem hiding this comment.
Who came up with this and why? Probably should be configurable.
There was a problem hiding this comment.
me - and yes itshoudl be but a flux of None causes it to fail so I had to give it some finite value
There was a problem hiding this comment.
or - alternatively we have a 'monitored' flag that allows us to do forced photometry on sources that we don't have fluxes for (i.e. asteroids or transients / dave's favorite sources)
There was a problem hiding this comment.
Uh what error does it raise? socat.db.ephem.create_ephem' should accept flux as None`.
Sulla2012
left a comment
Sulla2012
left a comment
There was a problem hiding this comment.
Looks mostly good except I'm confused about create_ephem raising an error if the flux is None. That's not expected behavior and we should fix it rather than patch it with some arbitrary default flux.
| rows.iterrows(), desc=f"Ingesting {designation}", total=len(rows) | ||
| ): | ||
| flux = None | ||
| flux = 200 * u.mJy # Default flux if not provided |
There was a problem hiding this comment.
Uh what error does it raise? socat.db.ephem.create_ephem' should accept flux as None`.
|
|
||
| The Simons Observatory source CATalog. Contains information about sources | ||
| that the Simons Observatory is monitorning. | ||
| The Simons Observatory source CATalog. Contains information about sources that the Simons Observatory is monitoring. |
There was a problem hiding this comment.
The SO in SOCAT is actually for SOurce
There was a problem hiding this comment.
create_ephem doesn't raise an error but somethign else did further down the line. It might have just been get_forced_photometry_sources or something. which I could have skirted by doing flux selection later
There was a problem hiding this comment.
Can you write a test that fails or otherwise give me something I can use to reproduce?
There was a problem hiding this comment.
ok, just added test -- the cause is in SourceGenerator
There was a problem hiding this comment.
Ah ok well I'll run this but almost certainly the problem is that flux is one of the interpolated values in SourceGenerator. It was asked that I add this feature to support the ability to filter sources by flux. This is then sort of a high level question of what we want to do with sources without flux estimates. We can simply return None for the flux value if the source in socat has no flux estimate [i.e., return (x,y, None)], or we can change the shape of the return type to be only (x,y) as opposed to (x,y,flux.
There was a problem hiding this comment.
I think we want to just return None for the flux; i.e. .at_time() would return (postition,None) and we don't have to worry about interpolating it.
We will have to decide what that means for forced photometry.
3 participants
add fluxes to ingested ssos and update time to be Astropydantic compatible.
fix source generator problem.
🤖 sqlite errors fixed by claude.
Claude said the problem I was having was:
Root cause: With your working coords, no sources are found so SourceGenerator objects are never created. With new coords that actually contain sources, get_box creates SourceGenerator(source=s, ...) without passing a session_factory. SourceGenerator.init then tries to open a fresh database connection via Settings().sync_database_url — but opening a second SQLite connection to the same file on Lustre fails with "unable to open database file."
Fix (2 changes to the installed socat/client/db.py):
SolarSystemClient.init now explicitly computes and stores self._session_factory before building the session interface
get_box passes session_factory=self._session_factory to each SourceGenerator, so they reuse the already-established connection factory instead of trying to open a new one